libvirt users - Mar 2018 - Re: Fail in virDomainUpdateDeviceFlags (libvirt-4.0.0 + Qemu-kvm 2.9.0 + Ceph 10.2.10)

On Tue, Feb 27, 2018 at 09:53:00 +0100, Michal Privoznik
wrote:
> On 02/27/2018 03:06 AM, Star Guo wrote:
> > Hello Everyone,
> > 
> >  
> > 
> > My pc run in CentOS 7.4 and install libvirt-4.0.0 + Qemu-kvm 2.9.0 +
Ceph
> > 10.2.10 ALL-in-One.
> > 
> >  
> > 
> > I use python-sdk with libvirt and run
[self.domain.updateDeviceFlags(xml,
> > libvirt.VIR_DOMAIN_AFFECT_LIVE)] on CDROM (I want to change media
path).
> > However, I enable libvirt debug log , the log as below:
> > 
> > <snip/>
> >
> > I see the flow is virDomainUpdateDeviceFlags ->
qemuMonitorChangeMedia, but
> > the cephx auth is drop, so make update error. Anybody meet this error?
> 
> Yes, this is a libvirt bug. I think this fixes the issue:
> 
> diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
> index 96454c17c..0e5ad9971 100644
> --- i/src/qemu/qemu_driver.c
> +++ w/src/qemu/qemu_driver.c
> @@ -7842,6 +7842,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
>                           virQEMUDriverPtr driver,
>                           bool force)
>  {
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>      virDomainDiskDefPtr disk = dev->data.disk;
>      virDomainDiskDefPtr orig_disk = NULL;
>      virDomainDeviceDef oldDev = { .type = dev->type };
> @@ -7850,6 +7852,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
>      if (virDomainDiskTranslateSourcePool(disk) < 0)
>          goto cleanup;
>  
> +    if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
> +        goto cleanup;
It's not that easy. At this point you also need to hotplug the
'secret'
object. Without that the command will fail as the secret object
referenced by the storage source definition will not be present.

There should be a upstream bugzilla tracking this and I'm planing to fix
this during my work on using the new blockdev stuff in qemu.

On 03/02/2018 08:28 AM, Peter Krempa wrote:
> On Tue, Feb 27, 2018 at 09:53:00 +0100, Michal Privoznik wrote:
>> On 02/27/2018 03:06 AM, Star Guo wrote:
>>> Hello Everyone,
>>>
>>>  
>>>
>>> My pc run in CentOS 7.4 and install libvirt-4.0.0 + Qemu-kvm 2.9.0
+ Ceph
>>> 10.2.10 ALL-in-One.
>>>
>>>  
>>>
>>> I use python-sdk with libvirt and run
[self.domain.updateDeviceFlags(xml,
>>> libvirt.VIR_DOMAIN_AFFECT_LIVE)] on CDROM (I want to change media
path).
>>> However, I enable libvirt debug log , the log as below:
>>>
>>> <snip/>
>>>
>>> I see the flow is virDomainUpdateDeviceFlags ->
qemuMonitorChangeMedia, but
>>> the cephx auth is drop, so make update error. Anybody meet this
error?
>>
>> Yes, this is a libvirt bug. I think this fixes the issue:
>>
>> diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
>> index 96454c17c..0e5ad9971 100644
>> --- i/src/qemu/qemu_driver.c
>> +++ w/src/qemu/qemu_driver.c
>> @@ -7842,6 +7842,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
>>                           virQEMUDriverPtr driver,
>>                           bool force)
>>  {
>> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>>      virDomainDiskDefPtr disk = dev->data.disk;
>>      virDomainDiskDefPtr orig_disk = NULL;
>>      virDomainDeviceDef oldDev = { .type = dev->type };
>> @@ -7850,6 +7852,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
>>      if (virDomainDiskTranslateSourcePool(disk) < 0)
>>          goto cleanup;
>>  
>> +    if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
>> +        goto cleanup;
> 
> It's not that easy. At this point you also need to hotplug the
'secret'
> object. Without that the command will fail as the secret object
> referenced by the storage source definition will not be present.
> 
Just because it may not be obvious that the thread forked (probably
email client for original poster)... Anyway, see what ended up being a
longer explanation here:

https://www.redhat.com/archives/libvirt-users/2018-February/msg00086.html

Where the short story is I don't think you need to hotplug a secret
object as that should already be there. One cannot hotplug a cdrom, so
the definition and the secret used must already exist. It's a strange
code path and while looking I didn't find a mechanism that would allow
an "empty" or "tray='open'" to actually create a
dummy entry.

I believe there needs to be a way via the (newer, but not supported in
libvirt) blockdev-change-medium qmp command to tell it what the secret
object id is.
> There should be a upstream bugzilla tracking this and I'm planing to
fix
> this during my work on using the new blockdev stuff in qemu.
> 
And yes, bzs would be good to track all this.

John

On Fri, Mar 02, 2018 at 15:32:44 -0500, John Ferlan
wrote:
> 
> 
> On 03/02/2018 08:28 AM, Peter Krempa wrote:
> > On Tue, Feb 27, 2018 at 09:53:00 +0100, Michal Privoznik wrote:
> >> On 02/27/2018 03:06 AM, Star Guo wrote:
> >>> Hello Everyone,
> >>>
> >>>  
> >>>
> >>> My pc run in CentOS 7.4 and install libvirt-4.0.0 + Qemu-kvm
2.9.0 + Ceph
> >>> 10.2.10 ALL-in-One.
> >>>
> >>>  
> >>>
> >>> I use python-sdk with libvirt and run
[self.domain.updateDeviceFlags(xml,
> >>> libvirt.VIR_DOMAIN_AFFECT_LIVE)] on CDROM (I want to change
media path).
> >>> However, I enable libvirt debug log , the log as below:
> >>>
> >>> <snip/>
> >>>
> >>> I see the flow is virDomainUpdateDeviceFlags ->
qemuMonitorChangeMedia, but
> >>> the cephx auth is drop, so make update error. Anybody meet
this error?
> >>
> >> Yes, this is a libvirt bug. I think this fixes the issue:
> >>
> >> diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
> >> index 96454c17c..0e5ad9971 100644
> >> --- i/src/qemu/qemu_driver.c
> >> +++ w/src/qemu/qemu_driver.c
> >> @@ -7842,6 +7842,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
> >>                           virQEMUDriverPtr driver,
> >>                           bool force)
> >>  {
> >> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> >> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> >>      virDomainDiskDefPtr disk = dev->data.disk;
> >>      virDomainDiskDefPtr orig_disk = NULL;
> >>      virDomainDeviceDef oldDev = { .type = dev->type };
> >> @@ -7850,6 +7852,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
> >>      if (virDomainDiskTranslateSourcePool(disk) < 0)
> >>          goto cleanup;
> >>  
> >> +    if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
> >> +        goto cleanup;
> > 
> > It's not that easy. At this point you also need to hotplug the
'secret'
> > object. Without that the command will fail as the secret object
> > referenced by the storage source definition will not be present.
> > 
> 
> Just because it may not be obvious that the thread forked (probably
> email client for original poster)... Anyway, see what ended up being a
> longer explanation here:
> 
> https://www.redhat.com/archives/libvirt-users/2018-February/msg00086.html
> 
> Where the short story is I don't think you need to hotplug a secret
> object as that should already be there. One cannot hotplug a cdrom, so
> the definition and the secret used must already exist. It's a strange
No, no, no. That is plain wrong. The secret object is property of the
image and not the drive. If you change the image you need to unplug all
old encryption and authentication secrets and plug back new ones.

That was the reason why I insisted so much that you actually make it a
member of virStorageSource.
> code path and while looking I didn't find a mechanism that would allow
> an "empty" or "tray='open'" to actually create
a dummy entry.
> 
> I believe there needs to be a way via the (newer, but not supported in
> libvirt) blockdev-change-medium qmp command to tell it what the secret
> object id is.
The object id of the secret is added via blockdev-add.
Blockdev-change-medium then uses that node as a media for the drive.
> 
> > There should be a upstream bugzilla tracking this and I'm planing
to fix
> > this during my work on using the new blockdev stuff in qemu.
> > 
> 
> And yes, bzs would be good to track all this.
> 
> John
>