Re: [PATCH v2 03/10] qga: treat get-guest-fsinfo as "best effort"

[Thomas Huth]

On 16/06/2022 16.43, John Snow wrote:
> On Thu, Jun 16, 2022 at 10:36 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> > Hi
> >
> > On Thu, Jun 16, 2022 at 6:27 PM John Snow <jsnow@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 <jsnow@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)
>
> Whoops, this is a debugging thing that I left in by accident. I was
> just so excited that after testing overnight, everything worked. :)
>
> > > +            fs->name = g_strdup("??\?-ENOENT");
> >
> >
> > Hmm, maybe we should make the field optional instead.
>
> Does that harm compatibility in a meaningful way? I'm happy to do
> whatever QGA maintainers want me to do. I just did something quick and
> dirty to get it working at all as a conversation starter. O:-)

Should the device get ignored instead of returning up a dummy device? ... at 
least that's what I'd expect at a quick glance at the problem...

 Thomas