On Thu, 4 Oct 2018 17:21:32 +0400 Marc-André Lureau <[email protected]> wrote:
> Hi > On Thu, Oct 4, 2018 at 3:22 PM Tomáš Golembiovský <[email protected]> wrote: > > > > The guest-get-fsinfo command collects also information about PCI > > controller where the disk is attached. When this fails for some reasons > > it tries to return just the partial information. However in certain > > cases the pointer to the structure was not initialized and was set to > > NULL. This breaks the serializer and leads to a crash of the guest agent. > > > > Signed-off-by: Tomáš Golembiovský <[email protected]> > > --- > > qga/commands-win32.c | 27 ++++++++++++++++++++++----- > > 1 file changed, 22 insertions(+), 5 deletions(-) > > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > > index 98d9735389..9c959122d9 100644 > > --- a/qga/commands-win32.c > > +++ b/qga/commands-win32.c > > @@ -633,15 +633,32 @@ static GuestDiskAddressList > > *build_guest_disk_info(char *guid, Error **errp) > > * > > https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx */ > > if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, > > scsi_ad, > > sizeof(SCSI_ADDRESS), &len, NULL)) { > > + Error *local_err = NULL; > > disk->unit = addr.Lun; > > disk->target = addr.TargetId; > > disk->bus = addr.PathId; > > - disk->pci_controller = get_pci_info(name, errp); > > + g_debug("unit=%lld target=%lld bus=%lld", > > + disk->unit, disk->target, disk->bus); > > + disk->pci_controller = get_pci_info(name, &local_err); > > + > > + if (local_err) { > > + g_debug("failed to get PCI controller info: %s", > > + error_get_pretty(local_err)); > > + error_free(local_err); > > + } else if (disk->pci_controller != NULL) { > > + g_debug("pci: domain=%lld bus=%lld slot=%lld > > function=%lld", > > + disk->pci_controller->domain, > > + disk->pci_controller->bus, > > + disk->pci_controller->slot, > > + disk->pci_controller->function); > > + } > > } > > - /* We do not set error in this case, because we still have enough > > - * information about volume. */ > > - } else { > > - disk->pci_controller = NULL; > > + } > > + /* We do not set error in case pci_controller is NULL, because we still > > + * have enough information about volume. */ > > + if (disk->pci_controller == NULL) { > > + g_debug("no PCI controller info"); > > + disk->pci_controller = g_malloc0(sizeof(GuestPCIAddress)); > > } > > Shouldn't pci-controller be made optional in the schema instead? It should, but that requires API change. Eric suggested that previously too: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08599.html I will do that in next version. Tomas > > > > > list = g_malloc0(sizeof(*list)); > > -- > > 2.19.0 > > -- Tomáš Golembiovský <[email protected]>
