GNU bug report logs - #42252
Not possible to reliably port forward with "guix system vm" anymore

Previous Next

Package: guix;

Reported by: Christopher Lemmer Webber <cwebber <at> dustycloud.org>

Date: Tue, 7 Jul 2020 20:41:01 UTC

Severity: normal

Done: Marius Bakke <marius <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42252 in the body.
You can then email your comments to 42252 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-guix <at> gnu.org:
bug#42252; Package guix. (Tue, 07 Jul 2020 20:41:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Christopher Lemmer Webber <cwebber <at> dustycloud.org>:
New bug report received and forwarded. Copy sent to bug-guix <at> gnu.org. (Tue, 07 Jul 2020 20:41:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Lemmer Webber <cwebber <at> dustycloud.org>
To: bug-guix <at> gnu.org
Subject: Not possible to reliably port forward with "guix system vm" anymore
Date: Tue, 07 Jul 2020 16:40:21 -0400
In commit 5379392731b52eef22b4936637eb592b93e04318, the following change
was introduced:

  modified   gnu/system/vm.scm
  @@ -941,6 +941,7 @@ with '-virtfs' options for the host file systems listed in SHARED-FS."
               '())
   
        "-no-reboot"
  +     "-nic" "user,model=virtio-net-pci"
        "-object" "rng-random,filename=/dev/urandom,id=guixsd-vm-rng"
        "-device" "virtio-rng-pci,rng=guixsd-vm-rng"

Unfortunately, this means that in our docs where we suggest doing the
following:

  `guix system vm config.scm` -nic user,model=virtio-net-pci,hostfwd=tcp::10022-:22

Since we now provide our own similar "-nic" field this creates a
*second* network interface at the same address and there is a race as in
terms of which handles connections.  Depending on the race result,
connections to the forwarded port may hang indefinitely.

Ironically, this regression was introduced to solve another regression!
From the commit message:

  This fixes a regression introduced in 8e53fe2b91d2776bc1529e7b34967c8f1d9edc32
  where 'guix system vm' would no longer be using virtio.

What's the right solution?  One could be that "guix system vm" itself
could take an argument that sets up port forwarding in the generated
shell script.  Eg:

  guix system vm config.scm --hostfwd=tcp::10022-:22 --hostfwd=tcp::8888-:80

kind of ugly, but it could work.  WDYT?

 - Chris




Information forwarded to bug-guix <at> gnu.org:
bug#42252; Package guix. (Tue, 07 Jul 2020 21:01:02 GMT) Full text and rfc822 format available.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Lemmer Webber <cwebber <at> dustycloud.org>
To: bug-guix <at> gnu.org
Subject: Re: Not possible to reliably port forward with "guix system vm"
 anymore
Date: Tue, 07 Jul 2020 17:00:02 -0400
Christopher Lemmer Webber writes:

>   guix system vm config.scm --hostfwd=tcp::10022-:22 --hostfwd=tcp::8888-:80
>
> kind of ugly, but it could work.  WDYT?

Kind of uglier, but more versatile:

  guix system vm config.scm --nic=user,model=virtio-net-pci,hostfwd=tcp::10022-:22,hostfwd=tcp::8888-:80




Information forwarded to bug-guix <at> gnu.org:
bug#42252; Package guix. (Tue, 07 Jul 2020 22:22:02 GMT) Full text and rfc822 format available.

Message #11 received at 42252 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Lemmer Webber <cwebber <at> dustycloud.org>
Cc: 42252 <at> debbugs.gnu.org, bug-guix <at> gnu.org
Subject: Re: bug#42252: Not possible to reliably port forward with "guix
 system vm" anymore
Date: Tue, 07 Jul 2020 18:21:16 -0400
[Message part 1 (text/plain, inline)]
Christopher Lemmer Webber writes:

> Christopher Lemmer Webber writes:
>
>>   guix system vm config.scm --hostfwd=tcp::10022-:22 --hostfwd=tcp::8888-:80
>>
>> kind of ugly, but it could work.  WDYT?
>
> Kind of uglier, but more versatile:
>
>   guix system vm config.scm --nic=user,model=virtio-net-pci,hostfwd=tcp::10022-:22,hostfwd=tcp::8888-:80

Here's a patch that implements just that.  Seems to work fine here!

[0001-guix-system-Add-nic-option-for-vm-subcommand.patch (text/x-patch, inline)]
From b008aad5d00fb6a87eb5a03d1849d6f52fb2307d Mon Sep 17 00:00:00 2001
From: Christopher Lemmer Webber <cwebber <at> dustycloud.org>
Date: Tue, 7 Jul 2020 18:13:29 -0400
Subject: [PATCH] guix system: Add --nic option for 'vm' subcommand.

This allows passing in explicit options for the qemu -nic keyword argument
for use in port forwarding, etc.

* (show-help, %options, process-action): Add and use command line
  argument of --nic for `guix system vm' and pass along appropriately.
  guix/scripts/system.scm (system-derivation-for-action):
  (perform-action): Define and use #:nic-options keyword.
* gnu/system/vm.scm (common-qemu-options)
  (system-qemu-image/shared-store-script): Likewise.
  (default-nic-options): New variable.
* doc/guix.texi (Running Guix in a VM): Use --nic argument in system vm port
  forwarding example.
---
 doc/guix.texi           |  2 +-
 gnu/system/vm.scm       | 16 ++++++++++++----
 guix/scripts/system.scm | 18 ++++++++++++++----
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 992bc303bb..4e76eb832a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -28315,7 +28315,7 @@ To enable SSH inside a VM you need to add an SSH server like
 22 by default, to the host.  You can do this with
 
 @example
-`guix system vm config.scm` -nic user,model=virtio-net-pci,hostfwd=tcp::10022-:22
+`guix system vm config.scm --nic=user,model=virtio-net-pci,hostfwd=tcp::10022-:22`
 @end example
 
 To connect to the VM you can run
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index f2b6b71b4d..78f89f6568 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -803,7 +803,11 @@ bootloader refers to: OS kernel, initrd, bootloader data, etc."
               #:register-closures? #f
               #:copy-inputs? full-boot?))
 
-(define* (common-qemu-options image shared-fs)
+(define default-nic-options
+  "user,model=virtio-net-pci")
+
+(define* (common-qemu-options image shared-fs
+                              #:key nic-options)
   "Return the a string-value gexp with the common QEMU options to boot IMAGE,
 with '-virtfs' options for the host file systems listed in SHARED-FS."
 
@@ -819,7 +823,9 @@ with '-virtfs' options for the host file systems listed in SHARED-FS."
             '())
 
      "-no-reboot"
-     "-nic" "user,model=virtio-net-pci"
+
+     ;; Construct -nic while forwarding all --hostfwd=... options
+     "-nic" #$(or nic-options default-nic-options)
      "-object" "rng-random,filename=/dev/urandom,id=guixsd-vm-rng"
      "-device" "virtio-rng-pci,rng=guixsd-vm-rng"
 
@@ -840,7 +846,8 @@ with '-virtfs' options for the host file systems listed in SHARED-FS."
                                                 (disk-image-size
                                                  (* (if full-boot? 500 70)
                                                     (expt 2 20)))
-                                                (options '()))
+                                                (options '())
+                                                (nic-options #f))
   "Return a derivation that builds a script to run a virtual machine image of
 OS that shares its store with the host.  The virtual machine runs with
 MEMORY-SIZE MiB of memory.
@@ -874,7 +881,8 @@ it is mostly useful when FULL-BOOT?  is true."
                                 (string-join #$kernel-arguments " "))))
               #$@(common-qemu-options image
                                       (map file-system-mapping-source
-                                           (cons %store-mapping mappings)))
+                                           (cons %store-mapping mappings))
+                                      #:nic-options nic-options)
               "-m " (number->string #$memory-size)
               #$@options))
 
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 61eeec622b..32cea9b184 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -684,7 +684,7 @@ checking this by themselves in their 'check' procedure."
 (define* (system-derivation-for-action os base-image action
                                        #:key image-size file-system-type
                                        full-boot? container-shared-network?
-                                       mappings)
+                                       mappings nic-options)
   "Return as a monadic value the derivation for OS according to ACTION."
   (case action
     ((build init reconfigure)
@@ -703,7 +703,8 @@ checking this by themselves in their 'check' procedure."
                                             (if full-boot?
                                                 image-size
                                                 (* 70 (expt 2 20)))
-                                            #:mappings mappings))
+                                            #:mappings mappings
+                                            #:nic-options nic-options))
     ((disk-image)
      (lower-object
       (system-image
@@ -764,6 +765,7 @@ and TARGET arguments."
                          use-substitutes? bootloader-target target
                          image-size file-system-type full-boot?
                          container-shared-network?
+                         nic-options
                          (mappings '())
                          (gc-root #f))
   "Perform ACTION for OS.  INSTALL-BOOTLOADER? specifies whether to install
@@ -819,7 +821,8 @@ static checks."
                                                 #:image-size image-size
                                                 #:full-boot? full-boot?
                                                 #:container-shared-network? container-shared-network?
-                                                #:mappings mappings))
+                                                #:mappings mappings
+                                                #:nic-options nic-options))
 
        ;; For 'init' and 'reconfigure', always build BOOTCFG, even if
        ;; --no-bootloader is passed, because we then use it as a GC root.
@@ -965,6 +968,8 @@ Some ACTIONS support additional ARGS.\n"))
       --share=SPEC       for 'vm', share host file system according to SPEC"))
   (display (G_ "
       --expose=SPEC      for 'vm', expose host file system according to SPEC"))
+  (display (G_ "
+      --nic=NIC-OPTIONS  for 'fm', set the value passed to qemu's -nic option"))
   (display (G_ "
   -N, --network          for 'container', allow containers to access the network"))
   (display (G_ "
@@ -1014,6 +1019,10 @@ Some ACTIONS support additional ARGS.\n"))
                  (lambda (opt name arg result)
                    (alist-cons 'image-size (size->number arg)
                                result)))
+         (option '("nic") #t #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'nic-options arg
+                               result)))
          (option '(#\N "network") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'container-shared-network? #t result)))
@@ -1172,7 +1181,8 @@ resulting from command-line parsing."
                                #:install-bootloader? bootloader?
                                #:target target-file
                                #:bootloader-target bootloader-target
-                               #:gc-root (assoc-ref opts 'gc-root)))))
+                               #:gc-root (assoc-ref opts 'gc-root)
+                               #:nic-options (assoc-ref opts 'nic-options)))))
           #:target target
           #:system system)))
     (warn-about-disk-space)))
-- 
2.26.2


Information forwarded to bug-guix <at> gnu.org:
bug#42252; Package guix. (Tue, 07 Jul 2020 22:39:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-guix <at> gnu.org:
bug#42252; Package guix. (Wed, 08 Jul 2020 09:47:01 GMT) Full text and rfc822 format available.

Message #17 received at 42252 <at> debbugs.gnu.org (full text, mbox):

From: Bengt Richter <bokr <at> bokr.com>
To: Christopher Lemmer Webber <cwebber <at> dustycloud.org>
Cc: 42252 <at> debbugs.gnu.org
Subject: Re: bug#42252: Not possible to reliably port forward with "guix
 system vm" anymore
Date: Wed, 8 Jul 2020 11:46:28 +0200
Hi

On +2020-07-07 16:40:21 -0400, Christopher Lemmer Webber wrote:
> In commit 5379392731b52eef22b4936637eb592b93e04318, the following change
> was introduced:
> 
>   modified   gnu/system/vm.scm
>   @@ -941,6 +941,7 @@ with '-virtfs' options for the host file systems listed in SHARED-FS."
>                '())
>    
>         "-no-reboot"
>   +     "-nic" "user,model=virtio-net-pci"
>         "-object" "rng-random,filename=/dev/urandom,id=guixsd-vm-rng"
>         "-device" "virtio-rng-pci,rng=guixsd-vm-rng"
> 
> Unfortunately, this means that in our docs where we suggest doing the
> following:
> 
>   `guix system vm config.scm` -nic user,model=virtio-net-pci,hostfwd=tcp::10022-:22
> 
> Since we now provide our own similar "-nic" field this creates a
> *second* network interface at the same address and there is a race as in
> terms of which handles connections.  Depending on the race result,
> connections to the forwarded port may hang indefinitely.
> 
> Ironically, this regression was introduced to solve another regression!
> From the commit message:
> 
>   This fixes a regression introduced in 8e53fe2b91d2776bc1529e7b34967c8f1d9edc32
>   where 'guix system vm' would no longer be using virtio.
>

This reminds a bit of doctors prescribing powerful medicine with side-effect so bad
that they have to prescribe a medicine for that, which in turn has side-effects,
in what I think is called prescription cascading, and people wind up on 25 pills a day.

"First, do no harm." :)

I wouldn't say anything, except ISTM your fix on top of a fix
is not the first to remind me of cascading :)

> What's the right solution?  One could be that "guix system vm" itself
> could take an argument that sets up port forwarding in the generated
> shell script.  Eg:
> 
>   guix system vm config.scm --hostfwd=tcp::10022-:22 --hostfwd=tcp::8888-:80
> 
> kind of ugly, but it could work.  WDYT?
> 
>  - Chris
> 
> 
> 

I'm not saying your solution is bad, I'm just saying cascading fixes may be a symptom
to diagnose, in case it indicates something like bad mutations involving bad genes
that will compromise the health of the guix ecology.

How is a "fix" judged with respect to the big picture?

Is there a higher level layered[1] design for the use of guix, like e.g. [2] which a proposed fix
might violate and therefore should be rejected, even though it makes something "work"?

Well, it's probably in an old paper by Ludo in some form, but I wonder
what concepts of layering guix developers are consciously using
when putting stuff between the declarations at the top and
the images at the bottom.

[1] https://en.wikipedia.org/wiki/Abstraction_layer
[2] https://en.wikipedia.org/wiki/OSI_model

-- 
Regards,
Bengt Richter
 




Information forwarded to bug-guix <at> gnu.org:
bug#42252; Package guix. (Sat, 11 Jul 2020 19:50:01 GMT) Full text and rfc822 format available.

Message #20 received at 42252 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Lemmer Webber <cwebber <at> dustycloud.org>
To: Bengt Richter <bokr <at> bokr.com>
Cc: 42252 <at> debbugs.gnu.org, Marius Bakke <marius <at> gnu.org>
Subject: Re: bug#42252: Not possible to reliably port forward with "guix
 system vm" anymore
Date: Sat, 11 Jul 2020 15:49:26 -0400
[+ Cc: Marius Bakke]
because I don't have enough info to respond fully myself.

Bengt Richter writes:

> Hi
>
> On +2020-07-07 16:40:21 -0400, Christopher Lemmer Webber wrote:
>> In commit 5379392731b52eef22b4936637eb592b93e04318, the following change
>> was introduced:
>> 
>>   modified   gnu/system/vm.scm
>>   @@ -941,6 +941,7 @@ with '-virtfs' options for the host file systems listed in SHARED-FS."
>>                '())
>>    
>>         "-no-reboot"
>>   +     "-nic" "user,model=virtio-net-pci"
>>         "-object" "rng-random,filename=/dev/urandom,id=guixsd-vm-rng"
>>         "-device" "virtio-rng-pci,rng=guixsd-vm-rng"
>> 
>> Unfortunately, this means that in our docs where we suggest doing the
>> following:
>> 
>>   `guix system vm config.scm` -nic user,model=virtio-net-pci,hostfwd=tcp::10022-:22
>> 
>> Since we now provide our own similar "-nic" field this creates a
>> *second* network interface at the same address and there is a race as in
>> terms of which handles connections.  Depending on the race result,
>> connections to the forwarded port may hang indefinitely.
>> 
>> Ironically, this regression was introduced to solve another regression!
>> From the commit message:
>> 
>>   This fixes a regression introduced in 8e53fe2b91d2776bc1529e7b34967c8f1d9edc32
>>   where 'guix system vm' would no longer be using virtio.
>>
>
> This reminds a bit of doctors prescribing powerful medicine with side-effect so bad
> that they have to prescribe a medicine for that, which in turn has side-effects,
> in what I think is called prescription cascading, and people wind up on 25 pills a day.
>
> "First, do no harm." :)

Well, I'm definitely not actively trying to harm ;)

> I wouldn't say anything, except ISTM your fix on top of a fix
> is not the first to remind me of cascading :)
>
>> What's the right solution?  One could be that "guix system vm" itself
>> could take an argument that sets up port forwarding in the generated
>> shell script.  Eg:
>> 
>>   guix system vm config.scm --hostfwd=tcp::10022-:22 --hostfwd=tcp::8888-:80
>> 
>> kind of ugly, but it could work.  WDYT?
>> 
>>  - Chris
>
> I'm not saying your solution is bad, I'm just saying cascading fixes may be a symptom
> to diagnose, in case it indicates something like bad mutations involving bad genes
> that will compromise the health of the guix ecology.
>
> How is a "fix" judged with respect to the big picture?

You raise a point in that my "fix to a fix" was a solution when I don't
fully understand the problem that was being fixed.  Of course, this
isn't uncommon in software development, but that doesn't make it great.
I only understood as much context as I could to make my workaround to
the problem.  Is it the right long-term solution?  I'm not sure, but
I think Marius Bakke has more context to be able to reply than I do.
What I do know is that the present instructions we have for port
forwarding are now effectively broken, and this at least provides a way
to get back there.  It might not be the right one.

For the rest of your email, I do think Guix is well layered... but even
well layered systems sometimes have intermediate proposed solutions to
bugs.  Sometimes the right design is found along the way.

But part of the goal of submitting such a patch is to get code review
and provoke discussion of what the right thing is.  I *did* make it
clear that I thought it was ugly, but workable.  :)

It might not be the totally wrong thing either though... if there are
enough modifications that users might make to the -nic flag, but we
don't know how to nicely abstract over all of them yet, but for now we
do need to supply a default, this can be an escape hatch at least for
now.  This wouldn't be uncommon; that's very similar to how Guix system
configuration tends to go (we supply configuration builders for the most
common options but sometimes provide a way to just slot in a manual
config file when need be).

 - Chris




Reply sent to Marius Bakke <marius <at> gnu.org>:
You have taken responsibility. (Sat, 11 Jul 2020 21:39:01 GMT) Full text and rfc822 format available.

Notification sent to Christopher Lemmer Webber <cwebber <at> dustycloud.org>:
bug acknowledged by developer. (Sat, 11 Jul 2020 21:39:02 GMT) Full text and rfc822 format available.

Message #25 received at 42252-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <marius <at> gnu.org>
To: Christopher Lemmer Webber <cwebber <at> dustycloud.org>,
 42252-done <at> debbugs.gnu.org
Subject: Re: bug#42252: Not possible to reliably port forward with "guix
 system vm" anymore
Date: Sat, 11 Jul 2020 23:38:26 +0200
[Message part 1 (text/plain, inline)]
Hello!

Sorry for this breakage, and thanks for the analysis!

Christopher Lemmer Webber <cwebber <at> dustycloud.org> writes:

> In commit 5379392731b52eef22b4936637eb592b93e04318, the following change
> was introduced:
>
>   modified   gnu/system/vm.scm
>   @@ -941,6 +941,7 @@ with '-virtfs' options for the host file systems listed in SHARED-FS."
>                '())
>    
>         "-no-reboot"
>   +     "-nic" "user,model=virtio-net-pci"
>         "-object" "rng-random,filename=/dev/urandom,id=guixsd-vm-rng"
>         "-device" "virtio-rng-pci,rng=guixsd-vm-rng"
>
> Unfortunately, this means that in our docs where we suggest doing the
> following:
>
>   `guix system vm config.scm` -nic user,model=virtio-net-pci,hostfwd=tcp::10022-:22
>
> Since we now provide our own similar "-nic" field this creates a
> *second* network interface at the same address and there is a race as in
> terms of which handles connections.  Depending on the race result,
> connections to the forwarded port may hang indefinitely.
>
> Ironically, this regression was introduced to solve another regression!
>>From the commit message:
>
>   This fixes a regression introduced in 8e53fe2b91d2776bc1529e7b34967c8f1d9edc32
>   where 'guix system vm' would no longer be using virtio.
>
> What's the right solution?  One could be that "guix system vm" itself
> could take an argument that sets up port forwarding in the generated
> shell script.  Eg:
>
>   guix system vm config.scm --hostfwd=tcp::10022-:22 --hostfwd=tcp::8888-:80
>
> kind of ugly, but it could work.  WDYT?

My motivation for the breaking commit was just that 'guix system vm' and
system tests would use virtio by default.  Without it, system tests with
forwarded ports used a different driver than those without forwardings.
It's a very minor issue and can be solved in other ways.  :-)

If no -nic parameter is specified on the QEMU command line, QEMU will
create one, emulating an Intel NIC.  I did not consider the discrepancy
this caused with the documentation when we unconditionally pass a -nic
parameter!

I think we should revert the commit, so that '`guix system vm` -nic foo'
works as expected for end users.  In fact I just did so.  :-)

Fixed in 1abf205d11c8b941d7d89855cb55a9cfde078838, thanks!
[signature.asc (application/pgp-signature, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 09 Aug 2020 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 232 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.