On Fri, Feb 24, 2017 at 03:57:23PM +0100, Pavel Grunt wrote:
> Theoretically a VM name can be a valid VM id or uuid. In that case
> connecting to the VMs may be problematic since virt-viewer selects
> the VM by its id then by uuid if not found then by its name.
>
> Introduce new command line options to cover this situation:
> "--domain-name" to connect to the VM by its name
> "--id" to connect to the VM by its id
> "--uuid" to connect to the VM by its uuid
> The options are mutually exclusive
>
> Resolves: rhbz#1399077
> ---
> man/virt-viewer.pod | 14 ++++++++
> src/virt-viewer.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 101 insertions(+), 10 deletions(-)
>
> diff --git a/man/virt-viewer.pod b/man/virt-viewer.pod
> index 9abf03c..30af8db 100644
> --- a/man/virt-viewer.pod
> +++ b/man/virt-viewer.pod
> @@ -122,6 +122,20 @@ kiosk-quit option to "on-disconnect" value, virt-viewer will quit
> instead. Please note that --reconnect takes precedence over this
> option, and will attempt to do a reconnection before it quits.
>
> +=item --id, --uuid, --domain-name
> +
> +Connect to the virtual machine by its id, uuid or name. These options
> +are mutual exclusive. For example the following command may sometimes
> +connect to a virtual machine with the id 2 or with the name 2 (depending
> +on the number of running machines):
> +
> + virt-viewer 2
> +
> +To always connect to the virtual machine with the name "2" use the
> +"--domain-name" option:
> +
> + virt-viewer --domain-name 2
> +
> =back
>
> =head1 CONFIGURATION
> diff --git a/src/virt-viewer.c b/src/virt-viewer.c
> index 1f99552..6bc9b6d 100644
> --- a/src/virt-viewer.c
> +++ b/src/virt-viewer.c
> @@ -82,6 +82,46 @@ static gboolean opt_attach = FALSE;
> static gboolean opt_waitvm = FALSE;
> static gboolean opt_reconnect = FALSE;
>
> +typedef enum {
> + DOMAIN_SELECTION_NONE,
> + DOMAIN_SELECTION_NAME,
> + DOMAIN_SELECTION_ID,
> + DOMAIN_SELECTION_UUID,
> +} DomainSelection;
> + switch (domain_selection_type) {
> + default: /* DOMAIN_SELECTION_NONE */
> + case DOMAIN_SELECTION_ID:
> + id = strtol(priv->domkey, &end, 10);
> + if (id >= 0 && end && !*end) {
> + dom = virDomainLookupByID(priv->conn, id);
> + }
> + if (domain_selection_type != DOMAIN_SELECTION_NONE) {
> + break;
> + }
> + /* fallthrough */
> + case DOMAIN_SELECTION_UUID:
> + if (!dom && virt_viewer_parse_uuid(priv->domkey, uuid) == 0) {
> + dom = virDomainLookupByUUID(priv->conn, uuid);
> + }
> + if (domain_selection_type != DOMAIN_SELECTION_NONE) {
> + break;
> + }
> + /* fallthrough */
> + case DOMAIN_SELECTION_NAME:
> + if (!dom) {
> + dom = virDomainLookupByName(priv->conn, priv->domkey);
> + }
> }
> +
> return dom;
> }
IMHO using a switch here isn't really helping particularly with the
tricks to allow fallthrough, except when you don't want to allow
fallthrough.
I think this can be made nicer with a different approach that uses
the enum as a bitfield. eg psuedo code...
int selection = 0;
if (--domain is set) {
selection |= DOMAIN_SELECTION_DOMAIN;
} else if (--uuid is set) {
selection |= DOMAIN_SELECTION_UUID;
} else if (--id is set) {
selection |= DOMAIN_SELECTION_ID;
}
if (!selection) {
selection = DOMAIN_SELECTION_DOMAIN |
DOMAIN_SELECTION_UUID |
DOMAIN_SELECTION_ID;
}
if (selection & DOMAIN_ID) {
dom = ...lookup by id...
if (dom)
selection = 0;
}
if (selection & DOMAIN_UUID) {
dom = ...lookup by uuid...
if (dom)
selection = 0;
}
if (selection & DOMAIN_NAME) {
dom = ...lookup by name...
if (dom)
selection = 0;
}
This also avoids the issue with the switch potentially generating
fallthrough warnings under gcc7.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
