On 01/17/2012 04:44 AM, Michal Privoznik wrote:
> Add a new API virDomainShutdownFlags and define:
>
> VIR_DOMAIN_SHUTDOWN_DEFAULT = 0,
> VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0),
> VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1),
>
> Also define some flags for the reboot API
>
> VIR_DOMAIN_REBOOT_DEFAULT = 0,
> VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0),
> VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1),
>
> Although these two APIs currently have the same flags, using
> separate enums allows them to expand separately in the future.
Fair enough.
>
> Add stub impls of the new API for all existing drivers
> ---
I might have split this into several patches, but I think doing it in
one go is okay since the stubs are trivial.
> include/libvirt/libvirt.h.in | 15 +++++++++
> src/driver.h | 5 +++
> src/libvirt.c | 65 +++++++++++++++++++++++++++++++++++++++++-
> src/libvirt_public.syms | 4 ++
That is, I might have done part 1 (the public API),
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 8 ++++-
> src/remote_protocol-structs | 5 +++
part 2 (the RPC),
> src/esx/esx_driver.c | 11 ++++++-
> src/libxl/libxl_driver.c | 12 +++++++-
> src/openvz/openvz_driver.c | 1 +
> src/test/test_driver.c | 11 ++++++-
> src/uml/uml_driver.c | 11 ++++++-
> src/vbox/vbox_tmpl.c | 11 ++++++-
> src/vmware/vmware_driver.c | 1 +
> src/xen/xen_driver.c | 14 ++++++++-
> src/xenapi/xenapi_driver.c | 12 +++++++-
and part 3 (the stubs).
> +++ b/include/libvirt/libvirt.h.in
> @@ -1148,7 +1148,22 @@ virDomainPtr virDomainLookupByUUID (virConnectPtr conn,
> virDomainPtr virDomainLookupByUUIDString (virConnectPtr conn,
> const char *uuid);
>
> +typedef enum {
> + VIR_DOMAIN_SHUTDOWN_DEFAULT = 0,
> + VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0),
> + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1),
> +} virDomainShutdownFlagValues;
No documentation comments? Even a /** doc */ prior to the typedef that
mentions virDomainShutdownFlagValues might be helpful to the doc
generation process.
> +++ b/src/libvirt.c
> @@ -3106,14 +3106,77 @@ error:
> }
>
> /**
> + * virDomainShutdownFlags:
> + * @domain: a domain object
> + * @flags: bitwise-OR of virDomainShutdownFlagValues
> + *
> + * Shutdown a domain, the domain object is still usable thereafter but
> + * the domain OS is being stopped. Note that the guest OS may ignore the
> + * request. For guests that react to a shutdown request, the differences
> + * from virDomainDestroy() are that the guest's disk storage will be in a
> + * stable state rather than having the (virtual) power cord pulled, and
> + * this command returns as soon as the shutdown request is issued rather
> + * than blocking until the guest is no longer running.
> + *
> + * If the domain is transient and has any snapshot metadata (see
> + * virDomainSnapshotNum()), then that metadata will automatically
> + * be deleted when the domain quits.
> + *
> + * If @flags is set to zero, then the hypervisor will chose the
> + * method of shutdown it considers best. To have greater control
> + * pass one of the virDomainShutdownFlagValues.
Maybe mention the existing flag names:
Use VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN to trigger a shutdown through an
ACPI interrupt, and VIR_DOMAIN_SHUTDOWN_GUEST_AGENT to trigger a
shutdown through a guest agent call (this requires that the domain have
a <channel> device appropriately wired to a guest agent).
Is it an error if multiple flags are requested at once?
> +/**
> * virDomainReboot:
> * @domain: a domain object
> - * @flags: extra flags; not used yet, so callers should always pass 0
> + * @flags: bitwise-OR of virDomainRebootFlagValues
> *
> * Reboot a domain, the domain object is still usable there after but
> * the domain OS is being stopped for a restart.
> * Note that the guest OS may ignore the request.
> *
> + * If @flags is set to zero, then the hypervisor will chose the
> + * method of shutdown it considers best. To have greater control
> + * pass one of the virDomainRebootFlagValues.
Again, mention the possible flag values, and that an agent request
requires additional support from the XML.
> +++ b/src/openvz/openvz_driver.c
> @@ -1693,6 +1693,7 @@ static virDriver openvzDriver = {
> .domainSuspend = openvzDomainSuspend, /* 0.8.3 */
> .domainResume = openvzDomainResume, /* 0.8.3 */
> .domainShutdown = openvzDomainShutdown, /* 0.3.1 */
> + .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */
Hmm. Are we setting ourselves up for issues down the road by sharing
shutdown and destroy implementations for openvz? But it's a
pre-existing problem, not made worse by the patch, and okay as long as
we reject all flag values.
> +++ b/src/vmware/vmware_driver.c
> @@ -980,6 +980,7 @@ static virDriver vmwareDriver = {
> .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */
> .domainResume = vmwareDomainResume, /* 0.8.7 */
> .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */
> + .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.10 */
> .domainReboot = vmwareDomainReboot, /* 0.8.7 */
> .domainDestroy = vmwareDomainShutdown, /* 0.8.7 */
> .domainDestroyFlags = vmwareDomainShutdownFlags, /* 0.9.4 */
Same potential issue as openvz. Oh well, not worth worrying about today.
ACK with documentation findings addressed.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list