On 01/23/2012 07:48 AM, Michal Privoznik wrote:
[for some reason, this one didn't get threaded properly - bug in git
send-email?]
This makes use of the QEMU guest agent to implement the
virDomainShutdownFlags and virDomainReboot APIs. With
no flags specified, it will prefer to use the agent, but
fallback to ACPI. Explicit choice can be made by using
a suitable flag
* src/qemu/qemu_driver.c: Wire up use of agent
---
src/qemu/qemu_driver.c | 131 ++++++++++++++++++++++++++++++++++++++---------
1 files changed, 106 insertions(+), 25 deletions(-)
-static int qemuDomainShutdown(virDomainPtr dom) {
+static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) {
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
qemuDomainObjPrivatePtr priv;
+ bool useAgent = false;
+
+ virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
+ VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
+
+ if (flags &
+ (VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN &
+ VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
Won't work. That statement is always false.
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Only one flag supported"));
+ return -1;
+ }
Back to my comment in 2/4, I'd prefer that you move this into libvirt.c
(so drivers don't have to duplicate it), where it should look like:
/* At most one of these two flags should be set. */
if ((flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) &&
(flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
> +
> + if (flags &
> + (VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN &
> + VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Only one flag supported"));
+ return -1;
+ }
Another instance of broken logic that will always be false, and which I
think belongs better in libvirt.c.
I didn't see anything else wrong in the patch, so if you fix patch 2,
then delete the two no-op hunks from patch 3, you have my ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org