7:08 a.m.
From: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
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.
Signed-off-by: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
---
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)))
+ return -1;
+
if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)
return -1;
--
2.27.0
10:11 p.m.
On a Wednesday in 2021, Masayoshi Mizuma wrote:
From: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
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
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.
Signed-off-by: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
---
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
12:35 a.m.
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(a)jp.fujitsu.com>
>
> 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(a)jp.fujitsu.com>
> ---
> 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
>
2:36 p.m.
On Wed, Aug 25, 2021 at 19:08:46 -0400, Masayoshi Mizuma wrote:
From: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
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.
Signed-off-by: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
---
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)))
+ return -1;
This should be in one of the validation functions checking support for
transient disks and with an appropriate error message.
1160
days inactive
1166
days old
3 comments
3 participants
participants (3)
-
Ján Tomko -
Masayoshi Mizuma -
Peter Krempa