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