On 10/20/2017 08:21 AM, Ján Tomko wrote:
> On Fri, Oct 20, 2017 at 08:03:29AM -0400, John Ferlan wrote:
>> Since qemuAssignDeviceDiskAlias checks whether the input info.alias
>> is already present, no need to check here as well.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_hotplug.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 51a7a68f84..9fbb3a0712 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -4447,10 +4447,8 @@
>> qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
>> goto cleanup;
>> }
>>
>> - if (!detach->info.alias) {
>> - if (qemuAssignDeviceDiskAlias(vm->def, detach,
>> priv->qemuCaps) < 0)
>> - goto cleanup;
>> - }
>> + if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
>> + goto cleanup;
>>
>
> All the calls assigning aliases in the Detach functions are
> unnecessary. At this point, all the domain's devices should have their
> aliases assigned. If by any case they do not, it is an error in other
> part of the libvirt code.
>
> I was going to send patches cleaning these up, but I could not decide
> whether to report an error if we find a device without an alias,
> or to just quietly assume that an alias. And I did not want to conflict
> with Michal's series again.
>
> Jan
>
I cycled back to this - if the alias was NULL and we're using json, then
virJSONValueObjectAddVArgs will fail w/ "argument key 'id'
must not have null value"; however, the text command path would fail in
qemuMonitorEscapeArg as soon as @in is deref'd.
So quietly assuming could end badly, but then again only for the old non
json path.
So either we report an error or just do what I did and rebuilt up the
alias. Is there a preference? IDC, either way.
Tks -
John
>> qemuDomainMarkDeviceForRemoval(vm, &detach->info);
>>
>> --
>> 2.13.6
>>
>> --
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
