Re: [Qemu-devel] [PATCH] pcibus_get_dev_path: correct pci device path construction

[Mark Cave-Ayland <mark.cave-ayland@siriusit.co.uk>]

On 18/01/11 21:10, Igor V. Kovalenko wrote:

> From: Igor V. Kovalenko<igor.v.kovalenko@gmail.com>
>
> - fix snprintf off by one
>    pci domain and slot number formatting snprintf calls
>    require extra space for trailing null character
>
>    without this change devices are assigned the same path name
>    which triggers assertion in vmstate_register_with_alias_id
>
> - while iterating over devices from root pci device
>    use PCI_SLOT and PCI_FUNC of each device on the path
>    instead of always extracting PCI_FUNC of original device
>
> Signed-off-by: Igor V. Kovalenko<igor.v.kovalenko@gmail.com>
> ---
>   hw/pci.c |   28 ++++++++++++++++++++--------
>   1 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 8d0e3df..182ee25 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -2036,6 +2036,8 @@ static char *pcibus_get_dev_path(DeviceState *dev)
>       int slot_len = strlen(":SS.F");
>       int path_len;
>       char *path, *p;
> +    PCIDevice** pci_path;
> +    int i;
>
>       /* Calculate # of slots on path between device and root. */;
>       slot_depth = 0;
> @@ -2045,21 +2047,31 @@ static char *pcibus_get_dev_path(DeviceState *dev)
>
>       path_len = domain_len + slot_len * slot_depth;
>
> -    /* Allocate memory, fill in the terminating null byte. */
> +    /* Allocate memory. String will be null-terminated by snprintf calls. */
>       path = malloc(path_len + 1 /* For '\0' */);
> -    path[path_len] = '\0';
>
>       /* First field is the domain. */
> -    snprintf(path, domain_len, "%04x:00", pci_find_domain(d->bus));
> +    snprintf(path, domain_len+1, "%04x:00", pci_find_domain(d->bus));
> +
> +    /* Store pci devices on the path walking up from device to root.
> +     * We need them later in the reverse order, last to first. */
> +    pci_path = qemu_malloc(slot_depth * sizeof(PCIDevice *));
>
> -    /* Fill in slot numbers. We walk up from device to root, so need to print
> -     * them in the reverse order, last to first. */
> -    p = path + path_len;
> +    i = slot_depth;
>       for (t = d; t; t = t->bus->parent_dev) {
> -        p -= slot_len;
> -        snprintf(p, slot_len, ":%02x.%x", PCI_SLOT(t->devfn), PCI_FUNC(d->devfn));
> +        pci_path[--i] = t;
>       }
>
> +    /* Fill in slot numbers using stored path from root pci device. */
> +    p = path + domain_len;
> +    for (i = 0; i<  slot_depth; ++i) {
> +        t = pci_path[i];
> +        snprintf(p + i * slot_len, slot_len+1,
> +                 ":%02x.%x", PCI_SLOT(t->devfn), PCI_FUNC(t->devfn));
> +    }
> +
> +    qemu_free(pci_path);
> +
>       return path;
>   }

Has anyone had a chance to look at this patch yet?


ATB,

Mark.

-- 
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063

Sirius Labs: http://www.siriusit.co.uk/labs