From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44730 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PjvBJ-00072U-Vz for qemu-devel@nongnu.org; Mon, 31 Jan 2011 09:57:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PjvB2-0002Sh-Tf for qemu-devel@nongnu.org; Mon, 31 Jan 2011 09:56:41 -0500 Received: from smtp.siriusit.co.uk ([81.187.123.5]:42316 helo=ra.servers.siriusit.co.uk) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PjvB2-0002RD-JR for qemu-devel@nongnu.org; Mon, 31 Jan 2011 09:56:24 -0500 Received: from [192.168.1.137] (mcavea@[192.168.1.137]) by ra.servers.siriusit.co.uk (8.13.3/8.13.3) with ESMTP id p0VEv84t018259 for ; Mon, 31 Jan 2011 14:57:08 GMT Message-ID: <4D46CD96.5090809@siriusit.co.uk> Date: Mon, 31 Jan 2011 14:56:22 +0000 From: Mark Cave-Ayland MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] pcibus_get_dev_path: correct pci device path construction References: <20110118211016.9850.63478.stgit@skyserv> In-Reply-To: <20110118211016.9850.63478.stgit@skyserv> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On 18/01/11 21:10, Igor V. Kovalenko wrote: > From: Igor V. Kovalenko > > - 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 > --- > 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