Re: [libvirt] [PATCH 03/10] qemu: Make basic upfront checks before creating command
On Tue, Feb 16, 2016 at 19:44:13 -0500, John Ferlan wrote:
Create qemuBuildPreCheck to make some basic upfront checks before
trying to build the command.
Unfortunately the 'spice' count was used later on, so yuck, handle that.
The count is not needed. Just the fact that there are spice devices is
required. Additionally I don't understand why spice-transported serial
ports are skipped rather than pointed out if there is no spice graphics
to transport them. I'd fix that one first rather than having to deal
with the pointer and having a checker that needs to leak stuff out to
the caller.
This will move some logic from much later to much earlier - we shouldn't
be adjusting any data so that shouldn't matter.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 169 +++++++++++++++++++++++++++---------------------
1 file changed, 97 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ab27619..36fe110 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6593,6 +6593,99 @@ qemuBuildTPMCommandLine(virDomainDefPtr def,
}
+/*
+ * qemuBuildPreCheck:
I'd prefer qemuBuildCommandLineValidate
+ *
+ * Perform some basic configuration checks before taking the plunge.
And something either more descriptive or at least finishing the sentence
after 'checks'.
+ */
+static int
+qemuBuildPreCheck(virQEMUDriverPtr driver,
+ const virDomainDef *def,
+ int *spicecnt)
Checks should not leak any value.
+{
+ size_t i;
+ int sdl = 0;
+ int vnc = 0;
+ int spice = 0;
[...]
+
+
+ return 0;
+
+ error:
+ return -1;
Nothing to clean up, thus the label doesn't make sense.
+}
+
+
qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
.qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
};
[...]
@@ -6677,47 +6768,8 @@ qemuBuildCommandLine(virConnectPtr conn,
-
- for (i = 0; i < def->ngraphics; ++i) {
- switch (def->graphics[i]->type) {
- case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
- ++sdl;
- break;
- case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
- ++vnc;
- break;
- case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
- ++spice;
- break;
- }
- }
+ if (qemuBuildPrecCheck(driver, def, &spicecnt) < 0)
This won't compile.
+ goto error;
/*
* do not use boot=on for drives when not using KVM since this
Peter
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)