On Mon, Jul 20, 2020 at 01:01:32PM +0200, Thomas Huth wrote:
> We are going to support non-PCI devices soon. For this we need to split
> the generic GuestDiskAddress and GuestDiskAddressList memory allocation
> and chaining into a separate function first.
>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> qga/commands-posix.c | 65 ++++++++++++++++++++++++++++----------------
> 1 file changed, 41 insertions(+), 24 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 1a62a3a70d..cddbaf5c69 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -861,28 +861,30 @@ static int build_hosts(char const *syspath, char const
> *host, bool ata,
> return i;
> }
>
> -/* Store disk device info specified by @sysfs into @fs */
> -static void build_guest_fsinfo_for_real_device(char const *syspath,
> - GuestFilesystemInfo *fs,
> - Error **errp)
> +/*
> + * Store disk device info for devices on the PCI bus.
> + * Returns true if information has been stored, or false for failure.
> + */
> +static bool build_guest_fsinfo_for_pci_dev(char const *syspath,
> + GuestDiskAddress *disk,
> + GuestPCIAddress *pciaddr,
> + Error **errp)
> {
> unsigned int pci[4], host, hosts[8], tgt[3];
> int i, nhosts = 0, pcilen;
> - GuestDiskAddress *disk;
> - GuestPCIAddress *pciaddr;
> - GuestDiskAddressList *list = NULL;
> bool has_ata = false, has_host = false, has_tgt = false;
> char *p, *q, *driver = NULL;
> #ifdef CONFIG_LIBUDEV
> struct udev *udev = NULL;
> struct udev_device *udevice = NULL;
> #endif
> + bool ret = false;
>
> p = strstr(syspath, "/devices/pci");
> if (!p || sscanf(p + 12, "%*x:%*x/%x:%x:%x.%x%n",
> pci, pci + 1, pci + 2, pci + 3, &pcilen) < 4) {
> g_debug("only pci device is supported: sysfs path '%s'", syspath);
> - return;
> + return false;
> }
>
> p += 12 + pcilen;
> @@ -903,7 +905,7 @@ static void build_guest_fsinfo_for_real_device(char const
> *syspath,
> }
>
> g_debug("unsupported driver or sysfs path '%s'", syspath);
> - return;
> + return false;
> }
>
> p = strstr(syspath, "/target");
> @@ -929,18 +931,11 @@ static void build_guest_fsinfo_for_real_device(char
> const *syspath,
> }
> }
>
> - pciaddr = g_malloc0(sizeof(*pciaddr));
> pciaddr->domain = pci[0];
> pciaddr->bus = pci[1];
> pciaddr->slot = pci[2];
> pciaddr->function = pci[3];
>
> - disk = g_malloc0(sizeof(*disk));
> - disk->pci_controller = pciaddr;
> -
> - list = g_malloc0(sizeof(*list));
> - list->value = disk;
> -
> #ifdef CONFIG_LIBUDEV
> udev = udev_new();
> udevice = udev_device_new_from_syspath(udev, syspath);
> @@ -1018,21 +1013,43 @@ static void build_guest_fsinfo_for_real_device(char
> const *syspath,
> goto cleanup;
> }
>
> - list->next = fs->disk;
> - fs->disk = list;
> - goto out;
> + ret = true;
>
> cleanup:
> - if (list) {
> - qapi_free_GuestDiskAddressList(list);
> - }
> -out:
> g_free(driver);
> #ifdef CONFIG_LIBUDEV
> udev_unref(udev);
> udev_device_unref(udevice);
> #endif
> - return;
> + return ret;
> +}
> +
> +/* Store disk device info specified by @sysfs into @fs */
> +static void build_guest_fsinfo_for_real_device(char const *syspath,
> + GuestFilesystemInfo *fs,
> + Error **errp)
> +{
> + GuestDiskAddress *disk;
> + GuestPCIAddress *pciaddr;
> + GuestDiskAddressList *list = NULL;
> + bool has_pci;
> +
> + pciaddr = g_malloc(sizeof(*pciaddr));
g_new0 instead of g_malloc and thus kill the sizeof.
> + memset(pciaddr, -1, sizeof(*pciaddr)); /* -1 means field is invalid */
Each field in GuestPCIAddress is an "int64_t", but memset works on bytes.
So you're not setting the fields to "-1" here, you're setting
each octet in the "int64_t" to -1.
> +
> + disk = g_malloc0(sizeof(*disk));
> + disk->pci_controller = pciaddr;
> +
> + list = g_malloc0(sizeof(*list));
> + list->value = disk;
g_new0 for these too.
(yes, I realize these were all pre-existing bugs)
> +
> + has_pci = build_guest_fsinfo_for_pci_dev(syspath, disk, pciaddr, errp);
> + if (has_pci) {
> + list->next = fs->disk;
> + fs->disk = list;
> + } else {
> + qapi_free_GuestDiskAddressList(list);
> + }
> }
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 :|
