On Thu, Aug 26, 2021 at 04:11:15PM +0200, Ján Tomko wrote: > On a Wednesday in 2021, Masayoshi Mizuma wrote: > > From: Masayoshi Mizuma <m.mizuma@xxxxxxxxxxxxxx> > > > > The VM is terminated abnormally when <transient shareBacking='yes'/> > > is set to the disk option and the qemu doesn't have set-action capability. > > > > # virsh start guest01 > > error: Failed to start domain 'guest01' > > error: internal error: qemu unexpectedly closed the monitor > > > > # > > > > Add checking the capability before system_reset QMP command is sent > > so that the VM can stop correctly when the qemu doesn't have the cap. > > > > From the commit message it's hard to see the connection between missing > QEMU_CAPS_SET_ACTION and not calling the system_reset command, since > the command was present long before set-action. > > Is this the same issue? > https://listman.redhat.com/archives/libvir-list/2021-July/msg00119.html I'm sorry I didn't make it clear enough. It's a similar issue, but it isn't the same issue. This patch tried to fix a new issue which was introduced the lifecycle event series. > > If I understand correctly, even after Peter's lifecycle event series > we cannot support the combination of: > * -no-reboot on the command line (which is still done for QEMUs without > SET_ACTION, i.e. older than 6.0) > * transient shareBacking disks > > In that case, this can be rejected much sooner in qemuValidate, before > even trying to start the QEMU process. That seems a great idea, thanks! I'll post the v2 to add check in the validation. Like as follows: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 8906aa52d9..47f12944aa 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3033,6 +3033,13 @@ qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk, } if (disk->transientShareBacking == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SET_ACTION)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("transiend disk backing image sharing isn't supported by this QEMU binary (%s)"), + disk->dst); + return -1; + } + /* sharing the backing file requires hotplug of the disk in the qemu driver */ switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_USB: --- - Masa > > > Signed-off-by: Masayoshi Mizuma <m.mizuma@xxxxxxxxxxxxxx> > > --- > > src/qemu/qemu_process.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > > index 3b4af61bf8..4bedd04679 100644 > > --- a/src/qemu/qemu_process.c > > +++ b/src/qemu/qemu_process.c > > @@ -7010,6 +7010,9 @@ qemuProcessSetupDisksTransientHotplug(virDomainObj *vm, > > if (hasHotpluggedDisk) { > > int rc; > > > > + if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION))) > > The error message is missing. > > Jano > > > + return -1; > > + > > if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) > > return -1; > > > > -- > > 2.27.0 > >
