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

[Laine Stump]

On 01/07/2014 05:20 PM, Eric Blake wrote:
> On 01/07/2014 07:53 AM, Laine Stump wrote:
>> 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.

Okay. I *think* that the bugzilla record and this email thread should be
enough that anyone encountering the problem will find the solution after
a quick google search. I don't know that putting it in libvirt release
notes would be of any use, since the release it *should* have been noted
in is long past...

>> +                    /* 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...

It's a bit of a moot point since we're not going to push it, but...

contAlias is just a const char * that normally would be a duplicate of
cont->info.alias. Since cont->info.alias is NULL, I create a temporary
alias in tempAlias and duplicate that instead.

>
>>                  }
>>                  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.

But by this time contAlias is already long ago out of scope.