Re: [Qemu-devel] [PATCH] qga: ignore non present cpus when handling qmp_guest_get_vcpus()

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 30 Aug 2018 17:51:13 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> +Drew
> 
> On 08/30/18 14:08, Igor Mammedov wrote:
> > If VM has VCPUs plugged sparselly (for example a VM started with
> > 3 VCPUs (cpu0, cpu1 and cpu2) and then cpu1 was hotunplugged so
> > only cpu0 and cpu2 are present), QGA will rise a error
> >   error: internal error: unable to execute QEMU agent command 'guest-get-vcpus':
> >   open("/sys/devices/system/cpu/cpu1/"): No such file or directory
> > when
> >   virsh vcpucount FOO --guest
> > is executed.
> > Fix it by ignoring non present CPUs when fetching CPUs status from sysfs.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  qga/commands-posix.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 37e8a2d..2929872 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2044,7 +2044,9 @@ static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
> >                                vcpu->logical_id);
> >      dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
> >      if (dirfd == -1) {
> > -        error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
> > +        if (!(sys2vcpu && errno == ENOENT)) {
> > +            error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
> > +        }
> >      } else {
> >          static const char fn[] = "online";
> >          int fd;
> >   
[...]
 
> I wonder if, instead of this patch, we should rework
> qmp_guest_get_vcpus(), to silently skip processors for which this
> dirpath ENOENT condition arises (i.e., return a shorter list of
> GuestLogicalProcessor objects).
would something like this on top of this patch do?

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 2929872..990bb80 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2114,12 +2114,14 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
         vcpu->logical_id = current++;
         vcpu->has_can_offline = true; /* lolspeak ftw */
         transfer_vcpu(vcpu, true, &local_err);
-
-        entry = g_malloc0(sizeof *entry);
-        entry->value = vcpu;
-
-        *link = entry;
-        link = &entry->next;
+        if (errno == ENOENT) {
+            g_free(vcpu);
+        } else {
+            entry = g_malloc0(sizeof *entry);
+            entry->value = vcpu;
+            *link = entry;
+            link = &entry->next;
+        }
     }
 
     if (local_err == NULL) {

[...]