[libvirt-users] Fail in virDomainUpdateDeviceFlags (libvirt-4.0.0 + Qemu-kvm 2.9.0 + Ceph 10.2.10)

Post by Star Guo
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).
<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;
+
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
goto cleanup;

@@ -7898,6 +7903,7 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,

ret = 0;
cleanup:
+ virObjectUnref(cfg);
return ret;
}

Can you check and confirm?

Michal

Peter Krempa

2018-03-02 13:28:40 UTC

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.

John Ferlan

2018-03-02 20:32:44 UTC

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.

Post by Peter Krempa
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

Peter Krempa

2018-03-05 08:47:26 UTC

Just because it may not be obvious that the thread forked (probably
email client for original poster)... Anyway, see what ended up being a
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.

Post by John Ferlan
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.

Post by Peter Krempa
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

John Ferlan

2018-03-05 16:11:37 UTC

Well... Kind of... Perhaps it's a matter of how I phrased things. Yes, I
agree the secret is associated with the image and not the drive;
however, the image is part of the drive command. So while technically
there is a split, my description didn't make that distinction mainly
because when it comes to the change commands all that matters is the
"filename" (or target) that's being changed and not the server. Since,
the authentication is to/for the server and changing the "filename" does
not make it possible to change the "server", just which file on the
server is being used.

Thus, even if you unplugged the old encryption and authentication secret
and plugged a new one in, AFAICT the "blockdev-change-medium" command
provides no mechanism to use the secrets. There is no parsing of server
and secret options.

Perhaps it's better to provide an example. Using an example since I have
iSCSI set up, when first started...

-object secret,id=ide0-0-1-secret0,...,
-drive file.driver=iscsi,file... {portal=, target=, lun=, transport=,
user=, file.password-secret=ide0-0-1-secret0}...,id=drive-ide0-0-1,...
-device ide-cd,bus=ide.0,...drive=drive-ide0-0-1,id=ide0-0-1

In order to open the iSCSI (or RBD) device the "file.password-secret"
command option of the "-drive" command describes the secret object id to
be used during the backend driver open (e.g. target of bdrv_file_open) -
using the example above "ide0-0-1-secret0". It's not the -drive command
id/alias (or "drive-ide0-0-1"). The file that gets opened on the server
would be the "file.target=" value.

The "change" command passes the device or drive alias
("drive-ide0-0-1"), the target or new source path, and an arg for
format. Internally to qemu, the change command "converts" into the
blockdev-change-medium.

The "blockdev-change-medium" takes similar options, but notes that the
device (e.g. drive alias) is deprecated in favor of an "id=" option
(although it still accepts the device if provided). AFAICT, the "id"
option is not used in libvirt yet. The block_set_io_throttle being the
example I looked at.

Following the code for using the "id" option, I see no connection to the
secret object which would store the server passphrase.

So when the bdrv_open is called from qmp_blockdev_change_medium, there
is no way to "connect" to the alias.

If it happens differently than that, then please show me what I'm missing.

Post by Peter Krempa
That was the reason why I insisted so much that you actually make it a
member of virStorageSource.

The object id of the secret is added via blockdev-add.
Blockdev-change-medium then uses that node as a media for the drive.

The former I see; however, the latter I'm not seeing how that happens.

John

Post by Peter Krempa
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

Peter Krempa

2018-03-05 16:39:50 UTC