Re: [libvirt] [PATCH v3 6/9] qemu_command: Honour chardev alias assignment with a function

On Tue, Jul 02, 2013 at 05:53:05PM +0200, Michal Privoznik wrote: > The chardev alias assignment is going to be needed in a separate > places, so it should be moved into a separate function rather > than copying code randomly around. > --- > src/qemu/qemu_command.c | 75 +++++++++++++++++++++++++++++++++++++++++++------ > src/qemu/qemu_command.h | 3 ++ > 2 files changed, 70 insertions(+), 8 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index ba93233..903839f 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -892,6 +892,65 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller) > return 0; > } > > +int > +qemuAssignDeviceChrAlias(virDomainDefPtr def, > + virDomainChrDefPtr chr, > + ssize_t idx) > +{ > + const char *prefix = NULL; > + const char *prefix2 = NULL; > + > + switch ((enum virDomainChrDeviceType) chr->deviceType) { > + case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: > + prefix = "parallel"; > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: > + prefix = "serial"; > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: > + prefix = "console"; > + prefix2 = "serial"; > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: > + prefix = "channel"; > + break; > + > + case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: > + return -1; > + } > + > + if (idx == -1) { > + virDomainChrDefPtr **arrPtr; > + size_t *cntPtr; > + size_t i; > + idx = 0; > + > + virDomainChrGetDomainPtrs(def, chr, &arrPtr, &cntPtr); > + > + for (i = 0; i < *cntPtr; i++) { > + int thisidx; > + if (((thisidx = qemuDomainDeviceAliasIndex(&(*arrPtr)[i]->info, prefix)) < 0) && > + (prefix2 && > + (thisidx = qemuDomainDeviceAliasIndex(&(*arrPtr)[i]->info, prefix2)) < 0)) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Unable to determine device index for character device")); > + return -1; > + } > + if (thisidx >= idx) > + idx = thisidx + 1; > + } > + } The commit message describes this as a simple refactoring, but this if (idx== -1) {...} is all new functionality compared to what is being replaced. I'm not too sure that this logic is correct either when dealing with <console> with a 'serialXX' alias.