6:54 a.m.
See patch 4.
Peter Krempa (4):
qemuMonitorJSONBlockdevAdd: Refactor cleanup
qemuMonitorJSONBlockdevDel: Refactor cleanup
qemuMonitorBlockdevAdd: Take double pointer argument
qemu: hotplug: Fix handling of the 'copy-on-read' layer with blockdev
src/qemu/qemu_block.c | 14 ++------------
src/qemu/qemu_hotplug.c | 25 +++++++++++++++---------
src/qemu/qemu_monitor.c | 16 ++++++----------
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c | 37 +++++++++++++-----------------------
src/qemu/qemu_monitor_json.h | 2 +-
6 files changed, 39 insertions(+), 57 deletions(-)
--
2.24.1
6:54 a.m.
New subject: [PATCH 1/4] qemuMonitorJSONBlockdevAdd: Refactor cleanup
Use automatic variable freeing and get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index dc1fc310ca..0d66f32887 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8811,25 +8811,19 @@ int
qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon,
virJSONValuePtr props)
{
- virJSONValuePtr cmd;
- virJSONValuePtr reply = NULL;
- int ret = -1;
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
if (!(cmd = qemuMonitorJSONMakeCommandInternal("blockdev-add", props)))
return -1;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
- goto cleanup;
+ return -1;
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- virJSONValueFree(cmd);
- virJSONValueFree(reply);
- return ret;
+ return 0;
}
--
2.24.1
6:54 a.m.
New subject: [PATCH 2/4] qemuMonitorJSONBlockdevDel: Refactor cleanup
Use automatic variable freeing and get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 0d66f32887..95b5ab6968 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8831,9 +8831,8 @@ int
qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
const char *nodename)
{
- virJSONValuePtr cmd;
- virJSONValuePtr reply = NULL;
- int ret = -1;
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-del",
"s:node-name", nodename,
@@ -8841,17 +8840,12 @@ qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
return -1;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
- goto cleanup;
+ return -1;
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- virJSONValueFree(cmd);
- virJSONValueFree(reply);
- return ret;
+ return 0;
}
--
2.24.1
6:54 a.m.
New subject: [PATCH 3/4] qemuMonitorBlockdevAdd: Take double pointer argument
Modify qemuMonitorBlockdevAdd so that it takes a double pointer for the
@props argument so that it's cleared inside the call. This allows
writing cleaner callers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 14 ++------------
src/qemu/qemu_monitor.c | 16 ++++++----------
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c | 5 +++--
src/qemu/qemu_monitor_json.h | 2 +-
5 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 63116ef5f2..fa6c6f98e3 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1530,13 +1530,8 @@ static int
qemuBlockStorageSourceAttachApplyStorage(qemuMonitorPtr mon,
qemuBlockStorageSourceAttachDataPtr data)
{
- int rv;
-
if (data->storageProps) {
- rv = qemuMonitorBlockdevAdd(mon, data->storageProps);
- data->storageProps = NULL;
-
- if (rv < 0)
+ if (qemuMonitorBlockdevAdd(mon, &data->storageProps) < 0)
return -1;
data->storageAttached = true;
@@ -1563,13 +1558,8 @@ static int
qemuBlockStorageSourceAttachApplyFormat(qemuMonitorPtr mon,
qemuBlockStorageSourceAttachDataPtr data)
{
- int rv;
-
if (data->formatProps) {
- rv = qemuMonitorBlockdevAdd(mon, data->formatProps);
- data->formatProps = NULL;
-
- if (rv < 0)
+ if (qemuMonitorBlockdevAdd(mon, &data->formatProps) < 0)
return -1;
data->formatAttached = true;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ceedcd527a..463e24657a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4390,23 +4390,19 @@ qemuMonitorBlockdevCreate(qemuMonitorPtr mon,
* @mon: monitor object
* @props: JSON object describing the blockdev to add
*
- * Adds a new block device (BDS) to qemu. Note that @props is always consumed
- * by this function and should not be accessed after calling this function.
+ * Adds a new block device (BDS) to qemu. Note that *@props is consumed
+ * and set to NULL on success.
*/
int
qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
- virJSONValuePtr props)
+ virJSONValuePtr *props)
{
- VIR_DEBUG("props=%p (node-name=%s)", props,
- NULLSTR(virJSONValueObjectGetString(props, "node-name")));
+ VIR_DEBUG("props=%p (node-name=%s)", *props,
+ NULLSTR(virJSONValueObjectGetString(*props, "node-name")));
- QEMU_CHECK_MONITOR_GOTO(mon, error);
+ QEMU_CHECK_MONITOR(mon);
return qemuMonitorJSONBlockdevAdd(mon, props);
-
- error:
- virJSONValueFree(props);
- return -1;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index cca2cdcb27..6a6b8efaee 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1323,7 +1323,7 @@ int qemuMonitorBlockdevCreate(qemuMonitorPtr mon,
virJSONValuePtr props);
int qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
- virJSONValuePtr props);
+ virJSONValuePtr *props);
int qemuMonitorBlockdevDel(qemuMonitorPtr mon,
const char *nodename);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 95b5ab6968..815d17520a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8809,12 +8809,13 @@ qemuMonitorJSONBlockdevCreate(qemuMonitorPtr mon,
int
qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon,
- virJSONValuePtr props)
+ virJSONValuePtr *props)
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ virJSONValuePtr pr = g_steal_pointer(props);
- if (!(cmd = qemuMonitorJSONMakeCommandInternal("blockdev-add", props)))
+ if (!(cmd = qemuMonitorJSONMakeCommandInternal("blockdev-add", pr)))
return -1;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 61f5b0061d..fd2e09025e 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -597,7 +597,7 @@ int qemuMonitorJSONBlockdevCreate(qemuMonitorPtr mon,
ATTRIBUTE_NONNULL(1);
int qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon,
- virJSONValuePtr props)
+ virJSONValuePtr *props)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
--
2.24.1
6:54 a.m.
New subject: [PATCH 4/4] qemu: hotplug: Fix handling of the 'copy-on-read' layer with blockdev
My original implementation was completely broken because it attempted to
use object-add/del instead of blockdev-add/del.
https://bugzilla.redhat.com/show_bug.cgi?id=1798366
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 39bcb70f83..c840889968 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -680,6 +680,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autoptr(virJSONValue) corProps = NULL;
g_autofree char *corAlias = NULL;
+ bool corAdded = false;
bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
if (qemuDomainStorageSourceChainAccessAllow(driver, vm, disk->src) < 0)
@@ -692,9 +693,12 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
goto cleanup;
if (blockdev) {
- if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
- !(corProps = qemuBlockStorageGetCopyOnReadProps(disk)))
- goto cleanup;
+ if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) {
+ if (!(corProps = qemuBlockStorageGetCopyOnReadProps(disk)))
+ goto cleanup;
+
+ corAlias = g_strdup(QEMU_DOMAIN_DISK_PRIVATE(disk)->nodeCopyOnRead);
+ }
if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->src,
priv->qemuCaps)))
@@ -719,9 +723,12 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuBlockStorageSourceChainAttach(priv->mon, data) < 0)
goto exit_monitor;
- if (corProps &&
- qemuMonitorAddObject(priv->mon, &corProps, &corAlias) < 0)
- goto exit_monitor;
+ if (corProps) {
+ if (qemuMonitorBlockdevAdd(priv->mon, &corProps) < 0)
+ goto exit_monitor;
+
+ corAdded = true;
+ }
if (qemuDomainAttachExtensionDevice(priv->mon, &disk->info) < 0)
goto exit_monitor;
@@ -763,8 +770,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
return ret;
exit_monitor:
- if (corAlias)
- ignore_value(qemuMonitorDelObject(priv->mon, corAlias));
+ if (corAdded)
+ ignore_value(qemuMonitorBlockdevDel(priv->mon, corAlias));
qemuBlockStorageSourceChainDetach(priv->mon, data);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
@@ -4250,7 +4257,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (corAlias)
- ignore_value(qemuMonitorDelObject(priv->mon, corAlias));
+ ignore_value(qemuMonitorBlockdevDel(priv->mon, corAlias));
if (diskBackend)
qemuBlockStorageSourceChainDetach(priv->mon, diskBackend);
--
2.24.1
8:57 a.m.
On 2/6/20 9:54 AM, Peter Krempa wrote:
See patch 4.
Peter Krempa (4):
qemuMonitorJSONBlockdevAdd: Refactor cleanup
qemuMonitorJSONBlockdevDel: Refactor cleanup
qemuMonitorBlockdevAdd: Take double pointer argument
qemu: hotplug: Fix handling of the 'copy-on-read' layer with blockdev
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
src/qemu/qemu_block.c | 14 ++------------
src/qemu/qemu_hotplug.c | 25 +++++++++++++++---------
src/qemu/qemu_monitor.c | 16 ++++++----------
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c | 37 +++++++++++++-----------------------
src/qemu/qemu_monitor_json.h | 2 +-
6 files changed, 39 insertions(+), 57 deletions(-)
11:07 a.m.
On Thu, Feb 06, 2020 at 01:54:15PM +0100, Peter Krempa wrote:
See patch 4.
Peter Krempa (4):
qemuMonitorJSONBlockdevAdd: Refactor cleanup
qemuMonitorJSONBlockdevDel: Refactor cleanup
qemuMonitorBlockdevAdd: Take double pointer argument
qemu: hotplug: Fix handling of the 'copy-on-read' layer with blockdev
src/qemu/qemu_block.c | 14 ++------------
src/qemu/qemu_hotplug.c | 25 +++++++++++++++---------
src/qemu/qemu_monitor.c | 16 ++++++----------
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c | 37 +++++++++++++-----------------------
src/qemu/qemu_monitor_json.h | 2 +-
6 files changed, 39 insertions(+), 57 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1754
days inactive
1754
days old
6 comments
3 participants
participants (3)
-
Daniel Henrique Barboza -
Ján Tomko -
Peter Krempa