[libvirt] [PATCH] Two safer versions of virDomainGetID
Two alternative versions of a safer virDomainGetID call in followups to this message. 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
This adds virDomainGetID2 which uses a pointer to int parameter, allowing the -1 (non-running) domain ID to be returned safely. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Wed, Sep 03, 2008 at 05:18:16PM +0100, Richard W.M. Jones wrote:
This adds virDomainGetID2 which uses a pointer to int parameter, allowing the -1 (non-running) domain ID to be returned safely.
With the patch this time ... Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Wed, Sep 03, 2008 at 05:19:20PM +0100, Richard W.M. Jones wrote:
On Wed, Sep 03, 2008 at 05:18:16PM +0100, Richard W.M. Jones wrote:
This adds virDomainGetID2 which uses a pointer to int parameter, allowing the -1 (non-running) domain ID to be returned safely.
With the patch this time ...
I don't particularly like this because it is implying that an inactive domain has an ID number, which is not correct. I think we primarily have to make the documentation for the existing API clearer. If there's a compelling need for applications to be able to determine whether a domain has an ID, then I think an API explicitly answering that question would be preferrable. eg something like /* * Return a non-zero positive number if the domain is active * and has a valid ID. Return zero if the domain is inactive * and does not have any ID number. Return -1 upon error */ int virDomainHasID(virDomainPtr dom) { if (!VIR_IS_DOMAIN(domain)) { virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); return -1; } if (dom->id == -1) return 0; else return 1; } Regards, 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 :|
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. 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. 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
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 :|
On Wed, Sep 03, 2008 at 05:17:19PM +0100, Richard W.M. Jones wrote:
Two alternative versions of a safer virDomainGetID call in followups to this message.
Anyone would like to comment on these alternatives? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
participants (2)
-
Daniel P. Berrange -
Richard W.M. Jones