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. It kind of feels like this if() block should be a separate function "qemuGetNextChrDevIndex" or something like that.
+ + if (virAsprintf(&chr->info.alias, "%s%zd", prefix, idx) < 0) { + virReportOOMError(); + return -1; + } + + return 0; +}
int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -947,20 +1006,20 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) goto no_memory; } for (i = 0; i < def->nparallels; i++) { - if (virAsprintf(&def->parallels[i]->info.alias, "parallel%d", i) < 0) - goto no_memory; + if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0) + return -1; } for (i = 0; i < def->nserials; i++) { - if (virAsprintf(&def->serials[i]->info.alias, "serial%d", i) < 0) - goto no_memory; + if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0) + return -1; } for (i = 0; i < def->nchannels; i++) { - if (virAsprintf(&def->channels[i]->info.alias, "channel%d", i) < 0) - goto no_memory; + if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0) + return -1; } for (i = 0; i < def->nconsoles; i++) { - if (virAsprintf(&def->consoles[i]->info.alias, "console%d", i) < 0) - goto no_memory; + if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) + return -1; } for (i = 0; i < def->nhubs; i++) { if (virAsprintf(&def->hubs[i]->info.alias, "hub%d", i) < 0) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 2993448..e92c78a 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -260,6 +260,9 @@ int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef, int qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev, int idx); int qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller); int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, int idx); +int qemuAssignDeviceChrAlias(virDomainDefPtr def, + virDomainChrDefPtr chr, + ssize_t idx);
int qemuParseKeywords(const char *str,
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|