On Thu, Jun 16, 2022 at 06:35:44PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Thu, Jun 16, 2022 at 6:27 PM John Snow <js...@redhat.com> wrote:
>
> > In some container environments, there may be references to block devices
> > witnessable from a container through /proc/self/mountinfo that reference
> > devices we simply don't have access to in the container, and could not
> > provide information about.
> >
> > Instead of failing the entire fsinfo command, return stub information
> > for these failed lookups.
> >
> > This allows test-qga to pass under docker tests, which are in turn used
> > by the CentOS VM tests.
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > qga/commands-posix.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 0469dc409d4..5989d4dca9d 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -1207,7 +1207,13 @@ static void build_guest_fsinfo_for_device(char
> > const *devpath,
> >
> > syspath = realpath(devpath, NULL);
> > if (!syspath) {
> > - error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
> > + if (errno == ENOENT) {
> > + /* This devpath may not exist because of container config,
> > etc. */
> > + fprintf(stderr, "realpath(%s) returned NULL/ENOENT\n",
> > devpath);
> >
>
> qga uses g_critical() (except for some win32 code paths atm)
>
>
> > + fs->name = y
> >
>
> Hmm, maybe we should make the field optional instead.
In my own testing, this method is called in various scenarios.
Some example:
devpath==/sys/dev/block/253:0
syspath==/sys/devices/virtual/block/dm-0
=> fs->name == dm-0
devpath==/sys/devices/virtual/block/dm-0/slaves/nvme0n1p4
syspath==/sys/devices/pci0000:00/0000:00:1d.0/0000:02:00.0/nvme/nvme0/nvme0n1/nvme0n1p4
=> fs->name == nvme0n1p4
devpath==/sys/dev/block/259:2
syspath==/sys/devices/pci0000:00/0000:00:1d.0/0000:02:00.0/nvme/nvme0/nvme0n1/nvme0n1p2
=> fs->name == nvme0n1p2
We set fs->name from basename(syspath)
If the realpath call fails, we could use basename(devpath). That
would sometimes give the correct answer, and in other types it
would at least give the major:minor number, which an admin can
manually correlate if desired via /proc/partitions.
If we want to be really advanced, we could just open /proc/partitions
and resolve the proper name ourselves, but that's probably overkill
basename(sysfspath)
is better than g_strdup("??\?-ENOENT") IMHO
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
