[libvirt] [PATCH alternative 2/2] Change contract of virDomainGetID to make it safer

[Daniel P. Berrange]

On Wed, Sep 03, 2008 at 05:22:45PM +0100, Richard W.M. Jones wrote:
> This changes the contract of the existing virDomainGetID call so that
> it is guaranteed to return the ID provided that the @domain parameter
> is not NULL or corrupted.

Actually this isn't entirely an accurate description. Inactive domains
do not have an ID of -1. This is merely a sentinal we use internally.
Inactive domains simply do not have an ID at all. Thus we return the
-1 error code if it is asked for. This is why tools like virt-manager
virsh do not display '-1' for ID - they simply leave the space blank.

> This should be compatible with all preceeding versions of libvirt,
> since all they have ever done is to check the @domain parameter and
> return the dom->id field.
> 
> However it might not be forwards compatible with future versions: At
> the moment there is an odd distinction between the local and remote
> case.  In the local case, the dom->id field is set to -1 when the
> domain goes (mostly anyhow, not always).  In the remote case it is not
> set, because this is not known.
> 
> In practice, this never really matters.  All significant libvirt
> callers grab a new virDomain object from the remote end each time,
> thus getting an updated ID.  virDomain objects seem to be individually
> very short-lived.

It is however a bug that the remote driver does not update its
internally ID field after performing Create/Destroy operations
on VMs, because it knows the ID will have changed at this point.

> 
> Rich.
> 
> -- 
> Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
> Read my OCaml programming blog: http://camltastic.blogspot.com/
> Fedora now supports 64 OCaml packages (the OPEN alternative to F#)
> http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

> Index: src/libvirt.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/libvirt.c,v
> retrieving revision 1.155
> diff -u -r1.155 libvirt.c
> --- src/libvirt.c	2 Sep 2008 15:00:09 -0000	1.155
> +++ src/libvirt.c	3 Sep 2008 16:16:47 -0000
> @@ -1878,7 +1878,9 @@
>   *
>   * Get the hypervisor ID number for the domain
>   *
> - * Returns the domain ID number or (unsigned int) -1 in case of error
> + * Returns the domain ID number or (unsigned int) -1 if the domain is
> + * not running.  If @domain is NULL or its memory is corrupted
> + * then this can also return (unsigned int) -1.

I think it needs to make it clear that an inactive domain does
not have an ID of -1 - this is simply an error code indicating
the domain has not valid ID at this time.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|