Not possible to reliably port forward with "guix system vm" anymore

  • Done
  • quality assurance status badge
Details
3 participants
  • Bengt Richter
  • Christopher Lemmer Webber
  • Marius Bakke
Owner
unassigned
Submitted by
Christopher Lemmer Webber
Severity
normal

Debbugs page

Christopher Lemmer Webber wrote 5 years ago
(address . bug-guix@gnu.org)
87r1tnf496.fsf@dustycloud.org
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
Christopher Lemmer Webber wrote 5 years ago
(address . bug-guix@gnu.org)
87o8orf3cd.fsf@dustycloud.org
Christopher Lemmer Webber writes:

Toggle quote (4 lines)
> 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
Christopher Lemmer Webber wrote 5 years ago
Re: bug#42252: Not possible to reliably port forward with "guix system vm" anymore
87lfjvezkz.fsf@dustycloud.org
Christopher Lemmer Webber writes:

Toggle quote (10 lines)
> 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!
Bengt Richter wrote 5 years ago
(name . Christopher Lemmer Webber)(address . cwebber@dustycloud.org)(address . 42252@debbugs.gnu.org)
20200708094628.GA9946@LionPure
Hi

On +2020-07-07 16:40:21 -0400, Christopher Lemmer Webber wrote:
Toggle quote (29 lines)
> 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 :)

Toggle quote (13 lines)
> 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.


--
Regards,
Bengt Richter
Christopher Lemmer Webber wrote 5 years ago
(name . Bengt Richter)(address . bokr@bokr.com)
87tuyd96ih.fsf@dustycloud.org
[+ Cc: Marius Bakke]
because I don't have enough info to respond fully myself.

Bengt Richter writes:

Toggle quote (38 lines)
> 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 ;)

Toggle quote (19 lines)
> 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
Marius Bakke wrote 5 years ago
87lfjpd965.fsf@gnu.org
Hello!

Sorry for this breakage, and thanks for the analysis!

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

Toggle quote (36 lines)
> 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!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAl8KMVIACgkQoqBt8qM6
VPqYngf+KupcZWHqayN+HYOBw9nxfh5e08NR05ZY0TZIIXg6wgObVZqKZ/KBiOOP
QP3BWtVMrNNBVySi9HzQqI1V1j2tCcAWgD8luxoSqbTjX5+zUCZ4R/xzB8A++BLF
g29wqjAKmqzb46TQZXQIzJluCKVFeQvfLdGtI+dRR5zu/M6Yk8xVAqdet6dQ931k
LZiOkL/ryPhNexEYyAy0CKcFaSZg6AMS/7J/CV2JOrpL0WntsELQmIEoWFiMijxU
CYg0Zem1SZoieAU+8qygStqKEo0gVZLLG9/p+xMGF/ief0FHFhJ/+M7+fRtSajKo
YVwnTuD5utR2Y6DozZsLKVELIFeOLg==
=wuhF
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 42252@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 42252
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help