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@xxxxxxxxxx> > --- > 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list