Re: [PATCH] release PCI address only when we have ensured it successfully

[ghostwcy <ghostwcy@xxxxxxxxx>]

At 2011-4-27 14:43, Daniel Veillard write:
> On Tue, Apr 26, 2011 at 11:40:01AM +0800, Wen Congyang wrote:
> > Steps to reproduce this bug:
> > 1. # cat net.xml # 00:03.0 has been used
> >      <interface type='network'>
> >        <mac address='52:54:00:04:72:f3'/>
> >        <source network='default'/>
> >        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >      </interface>
> >
> > 2. # virsh attach-device vm1 net.xml
> >     error: Failed to attach device from net.xml
> >     error: internal error unable to reserve PCI address 0:0:3
> >
> > 3. # virsh attach-device vm1 net.xml
> >     error: Failed to attach device from net.xml
> >     error: internal error unable to execute QEMU command 'device_add': Device 'rtl8139' could not be initialized
> >
> > The reason of this bug is that: we can not reserve PCI address 0:0:3 because it has
> > been used, but we release PCI address when we reserve it failed.
> >
> > ---
> >   src/qemu/qemu_hotplug.c |   13 +++++++++++++
> >   1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > index b03f774..5fdb013 100644
> > --- a/src/qemu/qemu_hotplug.c
> > +++ b/src/qemu/qemu_hotplug.c
> > @@ -147,6 +147,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
> >       qemuDomainObjPrivatePtr priv = vm->privateData;
> >       char *devstr = NULL;
> >       char *drivestr = NULL;
> > +    bool releaseaddr = false;
> >
> >       for (i = 0 ; i<  vm->def->ndisks ; i++) {
> >           if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
> > @@ -163,6 +164,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
> >       if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> >           if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&disk->info)<  0)
> >               goto error;
> > +        releaseaddr = true;
> >           if (qemuAssignDeviceDiskAlias(disk, qemuCaps)<  0)
> >               goto error;
> >
> > @@ -221,6 +223,7 @@ error:
> >
> >       if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&&
> >           (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&&
> > +        releaseaddr&&
> >           qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&disk->info)<  0)
> >           VIR_WARN("Unable to release PCI address on %s", disk->src);
> >
> > @@ -242,6 +245,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
> >       const char* type = virDomainControllerTypeToString(controller->type);
> >       char *devstr = NULL;
> >       qemuDomainObjPrivatePtr priv = vm->privateData;
> > +    bool releaseaddr = false;
> >
> >       for (i = 0 ; i<  vm->def->ncontrollers ; i++) {
> >           if ((vm->def->controllers[i]->type == controller->type)&&
> > @@ -256,6 +260,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
> >       if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> >           if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&controller->info)<  0)
> >               goto cleanup;
> > +        releaseaddr = true;
> >           if (qemuAssignDeviceControllerAlias(controller)<  0)
> >               goto cleanup;
> >
> > @@ -288,6 +293,7 @@ cleanup:
> >       if ((ret != 0)&&
> >           qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&&
> >           (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&&
> > +        releaseaddr&&
> >           qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&controller->info)<  0)
> >           VIR_WARN0("Unable to release PCI address on controller");
> >
> > @@ -559,6 +565,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> >       int ret = -1;
> >       virDomainDevicePCIAddress guestAddr;
> >       int vlan;
> > +    bool releaseaddr = false;
> >
> >       if (!qemuCapsGet(qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
> >           qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > @@ -595,6 +602,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> >           qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&net->info)<  0)
> >           goto cleanup;
> >
> > +    releaseaddr = true;
> > +
> >       if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV)&&
> >           qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> >           vlan = -1;
> > @@ -694,6 +703,7 @@ cleanup:
> >       if ((ret != 0)&&
> >           qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&&
> >           (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&&
> > +        releaseaddr&&
> >           qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&net->info)<  0)
> >           VIR_WARN0("Unable to release PCI address on NIC");
> >
> > @@ -757,6 +767,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver,
> >       char *devstr = NULL;
> >       int configfd = -1;
> >       char *configfd_name = NULL;
> > +    bool releaseaddr = false;
> >
> >       if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1)<  0) {
> >           virReportOOMError();
> > @@ -771,6 +782,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver,
> >               goto error;
> >           if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&hostdev->info)<  0)
> >               goto error;
> > +        releaseaddr = true;
> >           if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) {
> >               configfd = qemuOpenPCIConfig(hostdev);
> >               if (configfd>= 0) {
> > @@ -823,6 +835,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver,
> >   error:
> >       if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&&
> >           (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&&
> > +        releaseaddr&&
> >           qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&hostdev->info)<  0)
> >           VIR_WARN0("Unable to release PCI address on host device");
>
>    The logic fo the fix sounds right, and it looks complete,
>
>    ACK,

Thanks, pushed.

> Daniel

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list