[libvirt] [PATCH libvirt-glib] gobject: add GVir.DomainShutdownFlags binding
--- libvirt-gobject/libvirt-gobject-domain.c | 2 +- libvirt-gobject/libvirt-gobject-domain.h | 13 +++++++++++++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index ba8e12b..d12ac97 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -509,7 +509,7 @@ gboolean gvir_domain_delete(GVirDomain *dom, /** * gvir_domain_shutdown: * @dom: the domain - * @flags: the flags + * @flags: the %GVirDomainShutdownFlags flags */ gboolean gvir_domain_shutdown(GVirDomain *dom, guint flags G_GNUC_UNUSED, diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 70e7e37..c61a2f5 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -116,6 +116,19 @@ typedef enum { GVIR_DOMAIN_XML_UPDATE_CPU = VIR_DOMAIN_XML_UPDATE_CPU, } GVirDomainXMLFlags; +/** + * GVirDomainShutdownFlags: + * @GVIR_DOMAIN_SHUTDOWN_NONE: No flags, hypervisor choice + * @GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN: Send ACPI event + * @GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT: Use guest agent + * + */ +typedef enum { + GVIR_DOMAIN_SHUTDOWN_NONE = 0, + GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN, + GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT = VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, +} GVirDomainShutdownFlags; + typedef struct _GVirDomainInfo GVirDomainInfo; struct _GVirDomainInfo { diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index cc602d3..fe3de97 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -177,6 +177,7 @@ LIBVIRT_GOBJECT_0.0.9 { LIBVIRT_GOBJECT_0.1.1 { global: + gvir_domain_shutdown_flags_get_type; gvir_domain_xml_flags_get_type; } LIBVIRT_GOBJECT_0.0.9; -- 1.7.10.4
On Thu, Jul 19, 2012 at 07:36:50PM +0200, Marc-André Lureau wrote:
--- libvirt-gobject/libvirt-gobject-domain.c | 2 +- libvirt-gobject/libvirt-gobject-domain.h | 13 +++++++++++++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index ba8e12b..d12ac97 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -509,7 +509,7 @@ gboolean gvir_domain_delete(GVirDomain *dom, /** * gvir_domain_shutdown: * @dom: the domain - * @flags: the flags + * @flags: the %GVirDomainShutdownFlags flags
libvirt is using this wording: « @flags: bitwise-OR of virDomainShutdownFlagValues » which I find more descriptive.
*/ gboolean gvir_domain_shutdown(GVirDomain *dom, guint flags G_GNUC_UNUSED,
Why not, but the 'flags' parameter is unused in gvir_domain_shutdown, you need something like this in addition to this patch: diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index e14190c..c3f5202 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -512,7 +512,7 @@ gboolean gvir_domain_delete(GVirDomain *dom, * @flags: the flags */ gboolean gvir_domain_shutdown(GVirDomain *dom, - guint flags G_GNUC_UNUSED, + guint flags, GError **err) { GVirDomainPrivate *priv; @@ -521,7 +521,7 @@ gboolean gvir_domain_shutdown(GVirDomain *dom, g_return_val_if_fail(err == NULL || *err == NULL, FALSE); priv = dom->priv; - if (virDomainShutdown(priv->handle) < 0) { + if (virDomainShutdownFlags(priv->handle, flags) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, 0, "Unable to shutdown domain"); ACK Christophe
diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 70e7e37..c61a2f5 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -116,6 +116,19 @@ typedef enum { GVIR_DOMAIN_XML_UPDATE_CPU = VIR_DOMAIN_XML_UPDATE_CPU, } GVirDomainXMLFlags;
+/** + * GVirDomainShutdownFlags: + * @GVIR_DOMAIN_SHUTDOWN_NONE: No flags, hypervisor choice + * @GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN: Send ACPI event + * @GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT: Use guest agent + * + */ +typedef enum { + GVIR_DOMAIN_SHUTDOWN_NONE = 0, + GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN, + GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT = VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, +} GVirDomainShutdownFlags; + typedef struct _GVirDomainInfo GVirDomainInfo; struct _GVirDomainInfo { diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index cc602d3..fe3de97 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -177,6 +177,7 @@ LIBVIRT_GOBJECT_0.0.9 {
LIBVIRT_GOBJECT_0.1.1 { global: + gvir_domain_shutdown_flags_get_type; gvir_domain_xml_flags_get_type; } LIBVIRT_GOBJECT_0.0.9;
-- 1.7.10.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hey, On Thu, Jul 19, 2012 at 07:36:50PM +0200, Marc-André Lureau wrote:
--- libvirt-gobject/libvirt-gobject-domain.c | 2 +- libvirt-gobject/libvirt-gobject-domain.h | 13 +++++++++++++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index ba8e12b..d12ac97 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -509,7 +509,7 @@ gboolean gvir_domain_delete(GVirDomain *dom, /** * gvir_domain_shutdown: * @dom: the domain - * @flags: the flags + * @flags: the %GVirDomainShutdownFlags flags */ gboolean gvir_domain_shutdown(GVirDomain *dom, guint flags G_GNUC_UNUSED, diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 70e7e37..c61a2f5 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -116,6 +116,19 @@ typedef enum { GVIR_DOMAIN_XML_UPDATE_CPU = VIR_DOMAIN_XML_UPDATE_CPU, } GVirDomainXMLFlags;
+/** + * GVirDomainShutdownFlags: + * @GVIR_DOMAIN_SHUTDOWN_NONE: No flags, hypervisor choice + * @GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN: Send ACPI event + * @GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT: Use guest agent + * + */ +typedef enum { + GVIR_DOMAIN_SHUTDOWN_NONE = 0,
I was looking again at this patch, and I was wondering why it's not doing this instead of using SHUTDOWN_NONE: diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index c61a2f5..248a75f 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -118,13 +118,13 @@ typedef enum { /** * GVirDomainShutdownFlags: - * @GVIR_DOMAIN_SHUTDOWN_NONE: No flags, hypervisor choice + * @GVIR_DOMAIN_SHUTDOWN_DEFAULT: hypervisor choice * @GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN: Send ACPI event * @GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT: Use guest agent * */ typedef enum { - GVIR_DOMAIN_SHUTDOWN_NONE = 0, + GVIR_DOMAIN_SHUTDOWN_DEFAULT = VIR_DOMAIN_SHUTDOWN_DEFAULT, GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN, GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT = VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, } GVirDomainShutdownFlags; Should I send a patch to send this before this API appears in a release? Christophe
+ GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN, + GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT = VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, +} GVirDomainShutdownFlags; + typedef struct _GVirDomainInfo GVirDomainInfo; struct _GVirDomainInfo { diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index cc602d3..fe3de97 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -177,6 +177,7 @@ LIBVIRT_GOBJECT_0.0.9 {
LIBVIRT_GOBJECT_0.1.1 { global: + gvir_domain_shutdown_flags_get_type; gvir_domain_xml_flags_get_type; } LIBVIRT_GOBJECT_0.0.9;
-- 1.7.10.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hey On Fri, Aug 3, 2012 at 12:20 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
On Thu, Jul 19, 2012 at 07:36:50PM +0200, Marc-André Lureau wrote:
--- libvirt-gobject/libvirt-gobject-domain.c | 2 +- libvirt-gobject/libvirt-gobject-domain.h | 13 +++++++++++++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index ba8e12b..d12ac97 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -509,7 +509,7 @@ gboolean gvir_domain_delete(GVirDomain *dom, /** * gvir_domain_shutdown: * @dom: the domain - * @flags: the flags + * @flags: the %GVirDomainShutdownFlags flags */ gboolean gvir_domain_shutdown(GVirDomain *dom, guint flags G_GNUC_UNUSED, diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 70e7e37..c61a2f5 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -116,6 +116,19 @@ typedef enum { GVIR_DOMAIN_XML_UPDATE_CPU = VIR_DOMAIN_XML_UPDATE_CPU, } GVirDomainXMLFlags;
+/** + * GVirDomainShutdownFlags: + * @GVIR_DOMAIN_SHUTDOWN_NONE: No flags, hypervisor choice + * @GVIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN: Send ACPI event + * @GVIR_DOMAIN_SHUTDOWN_GUEST_AGENT: Use guest agent + * + */ +typedef enum { + GVIR_DOMAIN_SHUTDOWN_NONE = 0,
I was looking again at this patch, and I was wondering why it's not doing this instead of using SHUTDOWN_NONE:
The counterpart GVirDomainStartFlags uses _NONE. Is this a "flawed" in libvirt API? Do we prefer to copy libvirt or do we want consitancy? I prefer constitancy. However, DEFAULT was perhaps a better name than NONE for default behaviour. I would suggest with your patch we add START_DEFAULT, (but keep START_NONE), for consistancy. Daniel should now what feels right, so I will follow his opinion -- Marc-André Lureau
On Fri, Aug 03, 2012 at 12:44:10PM +0200, Marc-André Lureau wrote:
The counterpart GVirDomainStartFlags uses _NONE. Is this a "flawed" in libvirt API? Do we prefer to copy libvirt or do we want consitancy? I prefer constitancy.
I don't think this is a flaw, it's just that '0' has a different meaning depending on the API. Sometimes 0 means 'no flag', ie no additional behaviour, in this 'default' case, it means that the behaviour will be equivalent to one of the flags, but we don't indicate which one, we let the hypervisor choose the default one. So having different names for the enum for the 0 value doesn't strike me as inconsistent as they indicate slightly different things. Christophe
On Fri, Aug 3, 2012 at 12:54 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Fri, Aug 03, 2012 at 12:44:10PM +0200, Marc-André Lureau wrote:
The counterpart GVirDomainStartFlags uses _NONE. Is this a "flawed" in libvirt API? Do we prefer to copy libvirt or do we want consitancy? I prefer constitancy.
I don't think this is a flaw, it's just that '0' has a different meaning depending on the API. Sometimes 0 means 'no flag', ie no additional behaviour, in this 'default' case, it means that the behaviour will be equivalent to one of the flags, but we don't indicate which one, we let the hypervisor choose the default one. So having different names for the enum for the 0 value doesn't strike me as inconsistent as they indicate slightly different things.
Yes, but in this particual case : virDomainShutdownFlagValues VIR_DOMAIN_SHUTDOWN_DEFAULT = 0, /* hypervisor choice */ (in libvirt Create means Launch or Start) virDomainCreateFlags VIR_DOMAIN_NONE = 0, /* Default behavior */ It would make sense if libvirt had VIR_DOMAIN_START_DEFAULT instead (the rest of the values have DOMAIN_START prefix too..), and we use _DEFAULT too for something that leaves libvirt hypervisor backend choose the behavior. -- Marc-André Lureau
participants (2)
-
Christophe Fergeau -
Marc-André Lureau