xenstore utils dropped support for -s in 4.15

xenstore utils dropped support for -s in 4.15

[Henrik Riomar]

Hi,

In Alpine and Debian (probably elsewhere as well), the -s flag removed in these two commits:
 * https://github.com/xen-project/xen/commit/fa06cb8c38832aafe597d719040ba1d216e367b8
 * https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f
is actually used in the init scripts.

On one of the systems I tested 4.15 on things just hangs due to this:
2222  open("/lib/libxentoolcore.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory)
2222  open("/usr/local/lib/libxentoolcore.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory)
2222  open("/usr/lib/libxentoolcore.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
2222  fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
2222  fstat(3, {st_mode=S_IFREG|0755, st_size=14072, ...}) = 0
2222  read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\240\20\0\0\0\0\0\0"..., 960) = 960
2222  mmap(NULL, 20480, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f9925c02000
2222  mmap(0x7f9925c03000, 4096, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED, 3, 0x1000) = 0x7f9925c03000
2222  mmap(0x7f9925c04000, 4096, PROT_READ, MAP_PRIVATE|MAP_FIXED, 3, 0x2000) = 0x7f9925c04000
2222  mmap(0x7f9925c05000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x2000) = 0x7f9925c05000
2222  close(3)                          = 0
2222  mprotect(0x7f9925c0f000, 4096, PROT_READ) = 0
2222  mprotect(0x7f9925ca6000, 4096, PROT_READ) = 0
2222  mprotect(0x7f9925c05000, 4096, PROT_READ) = 0
2222  mprotect(0x5601e65dc000, 4096, PROT_READ) = 0
2222  stat("/var/run/xenstored/socket", 0x7ffd95b9f080) = -1 ENOENT (No such file or directory)
2222  access("/dev/xen/xenbus", F_OK)   = 0
2222  stat("/dev/xen/xenbus", {st_mode=S_IFCHR|0600, st_rdev=makedev(0xa, 0x3e), ...}) = 0
2222  open("/dev/xen/xenbus", O_RDWR|O_LARGEFILE) = 3
2222  rt_sigaction(SIGPIPE, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f9925c59c4b}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
2222  write(3, "\2\0\0\0\0\0\0\0\0\0\0\0\2\0\0\0", 16) = 16
2222  write(3, "/\0", 2)                = 2
2222  read(3, 

Reverting the two commits mentioned above restores a booting system.

The -s flag was introduced in 2005 or so, so I guess you can say that dropping it should
at least be mentioned in the release notices, and an alternative be described, or
-s functionally should be brought back.

Thanks,
 Henrik

How its used in Debian:
https://salsa.debian.org/xen-team/debian-xen/-/blob/master/debian/xen-utils-common.xen.init#L246

How its used in Alpine:
https://git.alpinelinux.org/aports/tree/main/xen/xenstored.initd#n25

Re: xenstore utils dropped support for -s in 4.15

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 2893 bytes --]

On 11.04.21 00:02, Henrik Riomar wrote:
> Hi,
> 
> In Alpine and Debian (probably elsewhere as well), the -s flag removed in these two commits:
>   * https://github.com/xen-project/xen/commit/fa06cb8c38832aafe597d719040ba1d216e367b8
>   * https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f
> is actually used in the init scripts.
> 
> On one of the systems I tested 4.15 on things just hangs due to this:
> 2222  open("/lib/libxentoolcore.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory)
> 2222  open("/usr/local/lib/libxentoolcore.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory)
> 2222  open("/usr/lib/libxentoolcore.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
> 2222  fcntl(3, F_SETFD, FD_CLOEXEC)     = 0
> 2222  fstat(3, {st_mode=S_IFREG|0755, st_size=14072, ...}) = 0
> 2222  read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\240\20\0\0\0\0\0\0"..., 960) = 960
> 2222  mmap(NULL, 20480, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f9925c02000
> 2222  mmap(0x7f9925c03000, 4096, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED, 3, 0x1000) = 0x7f9925c03000
> 2222  mmap(0x7f9925c04000, 4096, PROT_READ, MAP_PRIVATE|MAP_FIXED, 3, 0x2000) = 0x7f9925c04000
> 2222  mmap(0x7f9925c05000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x2000) = 0x7f9925c05000
> 2222  close(3)                          = 0
> 2222  mprotect(0x7f9925c0f000, 4096, PROT_READ) = 0
> 2222  mprotect(0x7f9925ca6000, 4096, PROT_READ) = 0
> 2222  mprotect(0x7f9925c05000, 4096, PROT_READ) = 0
> 2222  mprotect(0x5601e65dc000, 4096, PROT_READ) = 0
> 2222  stat("/var/run/xenstored/socket", 0x7ffd95b9f080) = -1 ENOENT (No such file or directory)
> 2222  access("/dev/xen/xenbus", F_OK)   = 0
> 2222  stat("/dev/xen/xenbus", {st_mode=S_IFCHR|0600, st_rdev=makedev(0xa, 0x3e), ...}) = 0
> 2222  open("/dev/xen/xenbus", O_RDWR|O_LARGEFILE) = 3
> 2222  rt_sigaction(SIGPIPE, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f9925c59c4b}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
> 2222  write(3, "\2\0\0\0\0\0\0\0\0\0\0\0\2\0\0\0", 16) = 16
> 2222  write(3, "/\0", 2)                = 2
> 2222  read(3,
> 
> Reverting the two commits mentioned above restores a booting system.
> 
> The -s flag was introduced in 2005 or so, so I guess you can say that dropping it should
> at least be mentioned in the release notices, and an alternative be described, or
> -s functionally should be brought back.

The -s served exactly no purpose.

It was meant to force socket usage. A socket will be used anyway in
case xenstored is running in dom0, so specifying -s won't change
anything in this case. In case xenstored is running in a stubdom
specifying -s will result in the utility NOT working, as there is
no socket connection to xenstored available.


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: xenstore utils dropped support for -s in 4.15

[Henrik Riomar]

On Sun, 11 Apr 2021 07:34:16 +0200
Juergen Gross <jgross@suse.com> wrote:

> On 11.04.21 00:02, Henrik Riomar wrote:
> > Hi,
> > 
> > In Alpine and Debian (probably elsewhere as well), the -s flag removed in these two commits:
> >   * https://github.com/xen-project/xen/commit/fa06cb8c38832aafe597d719040ba1d216e367b8
> >   * https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f
> > is actually used in the init scripts.

> > Reverting the two commits mentioned above restores a booting system.
> > 
> > The -s flag was introduced in 2005 or so, so I guess you can say that dropping it should
> > at least be mentioned in the release notices, and an alternative be described, or
> > -s functionally should be brought back.
> 
> The -s served exactly no purpose.

yes its used by dists to check that the socket is actually accessible (without falling back to 
direct access to /dev/xen/xenbus).

> 
> It was meant to force socket usage. A socket will be used anyway in
> case xenstored is running in dom0, so specifying -s won't change
> anything in this case. 

yes reverting the to commits above and using -s, brings back the original behavior, returning
with failure if the socket is not there.

There are two issues here I think:
 1. dists actually use -s to check if the daemon is up (and been doing this for a long time)
 2. Reads of /dev/xen/xenbus (via xenstore-read -s /), blocks for ever in 4.15

/ Henrik

Re: xenstore utils dropped support for -s in 4.15

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 2013 bytes --]

On 11.04.21 09:17, Henrik Riomar wrote:
> On Sun, 11 Apr 2021 07:34:16 +0200
> Juergen Gross <jgross@suse.com> wrote:
> 
>> On 11.04.21 00:02, Henrik Riomar wrote:
>>> Hi,
>>>
>>> In Alpine and Debian (probably elsewhere as well), the -s flag removed in these two commits:
>>>    * https://github.com/xen-project/xen/commit/fa06cb8c38832aafe597d719040ba1d216e367b8
>>>    * https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f
>>> is actually used in the init scripts.
> 
>>> Reverting the two commits mentioned above restores a booting system.
>>>
>>> The -s flag was introduced in 2005 or so, so I guess you can say that dropping it should
>>> at least be mentioned in the release notices, and an alternative be described, or
>>> -s functionally should be brought back.
>>
>> The -s served exactly no purpose.
> 
> yes its used by dists to check that the socket is actually accessible (without falling back to
> direct access to /dev/xen/xenbus).

There are Xen configurations where the socket is never accessible,
as it is not existing.

>> It was meant to force socket usage. A socket will be used anyway in
>> case xenstored is running in dom0, so specifying -s won't change
>> anything in this case.
> 
> yes reverting the to commits above and using -s, brings back the original behavior, returning
> with failure if the socket is not there.

And returning failure when Xenstore is running fine, but not in dom0.

> There are two issues here I think:
>   1. dists actually use -s to check if the daemon is up (and been doing this for a long time)

This should be changed, as it is based on wrong assumptions.

>   2. Reads of /dev/xen/xenbus (via xenstore-read -s /), blocks for ever in 4.15

So you are basically saying that you'd like to have a test "is Xenstore
running", and this test should work with the "-s" flag.

I can look into that, but reverting the "access via socket" removal flag
is not the way to go.


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: xenstore utils dropped support for -s in 4.15

[Henrik Riomar]

On Sun, 11 Apr 2021 11:17:38 +0200
Juergen Gross <jgross@suse.com> wrote:

> On 11.04.21 09:17, Henrik Riomar wrote:
> > On Sun, 11 Apr 2021 07:34:16 +0200
> > Juergen Gross <jgross@suse.com> wrote:
> > 
> >> On 11.04.21 00:02, Henrik Riomar wrote:
> >>> Hi,
> >>>
> >>> In Alpine and Debian (probably elsewhere as well), the -s flag removed in these two commits:
> >>>    * https://github.com/xen-project/xen/commit/fa06cb8c38832aafe597d719040ba1d216e367b8
> >>>    * https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f
> >>> is actually used in the init scripts.
> > 
> >>> Reverting the two commits mentioned above restores a booting system.
> >>>
> >>> The -s flag was introduced in 2005 or so, so I guess you can say that dropping it should
> >>> at least be mentioned in the release notices, and an alternative be described, or
> >>> -s functionally should be brought back.
> >>
> >> The -s served exactly no purpose.
> > 
> > yes its used by dists to check that the socket is actually accessible (without falling back to
> > direct access to /dev/xen/xenbus).
> 
> There are Xen configurations where the socket is never accessible,
> as it is not existing.

sure, but this is in the start script for the daemon it self we use "xenstore-read -s /"
so it will also work if that script is used in a domain only running xenstored, right?

> 
> >> It was meant to force socket usage. A socket will be used anyway in
> >> case xenstored is running in dom0, so specifying -s won't change
> >> anything in this case.
> > 
> > yes reverting the to commits above and using -s, brings back the original behavior, returning
> > with failure if the socket is not there.
> 
> And returning failure when Xenstore is running fine, but not in dom0.

sure, -s means access via local socket so yes.

> 
> > There are two issues here I think:
> >   1. dists actually use -s to check if the daemon is up (and been doing this for a long time)
> 
> This should be changed, as it is based on wrong assumptions.

changed to what exactly? check for a pid file? that also only works locally just like
-s.

> 
> >   2. Reads of /dev/xen/xenbus (via xenstore-read -s /), blocks for ever in 4.15
> 
> So you are basically saying that you'd like to have a test "is Xenstore
> running", and this test should work with the "-s" flag.
> 
> I can look into that, but reverting the "access via socket" removal flag
> is not the way to go.
> 

Ok, so what should dists packaging 4.15 do? can the release notices be updated with this?

Thanks,
 Henrik