[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: auto-create pci controller alias if missing from domain status



On 01/07/2014 07:53 AM, Laine Stump wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1047429
> 
> The problem occurs when a host with a running guest is upgraded from
> pre-1.0.2 libvirt, and then a later attempt is made to hotplug a new
> PCI device (emulated or passthrough) to that guest without having
> power cycled / save-restored / migrated the guest. The hotplug fails
> with this error:
> 
>    internal error: Device alias was not set for PCI controller with
>             index 0 required for device at address 0000:00:xx.x
> 

> 
> I'm not 100% convinced that it's necessary to handle *this* case
> either (upgrading from pre-1.0.2), but thought I should send the patch
> to the list anyway to see what others think. My opinion is that it's
> creating permanent clutter in the code in order to handle a situation
> that will come up just once in the entire lifetime of a host (and even
> then only if libvirt is upgraded without somehow restarting a guest,
> and *then* a device is hotplugged to that guest), so it may be better
> if we just left the workaround ("restart/save-restore/migrate the
> domain if you need to hotplug a device") as a footnote somewhere
> instead.

Yes, I think I could live with such a footnote.  And even if we get
outvoted and someone else requests the permanent code clutter, we can
add that as a separate commit which gives the further justification of
why we are adding it.

> +                    /* this could be a case where the domain had been
> +                     * started with an earlier version of libvirt that
> +                     * had no device entry for the PCI bus
> +                     * (pre-1.0.2), and in that case no alias would be
> +                     * set in the status XML, so we create one using
> +                     * the standard "pci.N" pattern.
> +                     */
> +                    if (virAsprintf(&tempAlias, "pci.%u", info->addr.pci.bus) < 0)
> +                        goto cleanup;
> +                    contAlias = tempAlias;

contAlias and tempAlias point to the same location...

>                  }
>                  break;
>              }
> @@ -3118,6 +3123,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>      ret = 0;
>  cleanup:
>      VIR_FREE(devStr);
> +    VIR_FREE(tempAlias);

and you then free tempAlias.  That leaves contAlias pointing at stale
memory.  That doesn't feel right.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]