[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt-glib] gconfig: Add API to set domain/pm tree



On Wed, Oct 10, 2012 at 1:12 PM, Christophe Fergeau <cfergeau redhat com> wrote:
> On Tue, Oct 09, 2012 at 11:04:46PM +0300, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
>>
>> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-power-management.c b/libvirt-gconfig/libvirt-gconfig-domain-power-management.c
>> new file mode 100644
>> index 0000000..c6b1962
>> --- /dev/null
>> +++ b/libvirt-gconfig/libvirt-gconfig-domain-power-management.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * libvirt-gconfig-domain-power-management.c: libvirt domain power management configuration
>> + *
>> + * Copyright (C) 2012 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
>> + *
>> + * Authors:
>> + *
>> + * Zeeshan Ali (Khattak) <zeeshanak gnome org>
>> + * Christophe Fergeau <cfergeau redhat com>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include "libvirt-gconfig/libvirt-gconfig.h"
>> +#include "libvirt-gconfig/libvirt-gconfig-private.h"
>> +
>> +#define GVIR_CONFIG_DOMAIN_POWER_MANAGEMENT_GET_PRIVATE(obj) \
>> +        (G_TYPE_INSTANCE_GET_PRIVATE((obj),                  \
>> +         GVIR_CONFIG_TYPE_DOMAIN_POWER_MANAGEMENT,           \
>> +         GVirConfigDomainPowerManagementPrivate))
>> +
>> +struct _GVirConfigDomainPowerManagementPrivate
>> +{
>> +    gboolean unused;
>> +};
>> +
>> +G_DEFINE_TYPE(GVirConfigDomainPowerManagement,
>> +              gvir_config_domain_power_management,
>> +              GVIR_CONFIG_TYPE_OBJECT);
>> +
>> +static void gvir_config_domain_power_management_class_init
>> +        (GVirConfigDomainPowerManagementClass *klass)
>> +{
>> +    g_type_class_add_private(klass, sizeof(GVirConfigDomainPowerManagementPrivate));
>> +}
>> +
>> +
>> +static void
>> +gvir_config_domain_power_management_init(GVirConfigDomainPowerManagement *pm)
>> +{
>> +    g_debug("Init GVirConfigDomainPowerManagement=%p", pm);
>> +
>> +    pm->priv = GVIR_CONFIG_DOMAIN_POWER_MANAGEMENT_GET_PRIVATE(pm);
>> +}
>> +
>> +
>> +GVirConfigDomainPowerManagement *gvir_config_domain_power_management_new(void)
>> +{
>> +    GVirConfigObject *object;
>> +
>> +    object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_POWER_MANAGEMENT,
>> +                                    "pm", NULL);
>> +    return GVIR_CONFIG_DOMAIN_POWER_MANAGEMENT(object);
>> +}
>> +
>> +GVirConfigDomainPowerManagement *
>> +gvir_config_domain_power_management_new_from_xml(const gchar *xml,
>> +                                                 GError **error)
>> +{
>> +    GVirConfigObject *object;
>> +
>> +    object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_POWER_MANAGEMENT,
>> +                                             "pm", NULL, xml, error);
>> +    return GVIR_CONFIG_DOMAIN_POWER_MANAGEMENT(object);
>> +}
>> +
>> +void gvir_config_domain_power_management_set_ram_suspend_enabled
>> +        (GVirConfigDomainPowerManagement *pm, gboolean enabled)
>
> It's generally indented as
> void gvir_config_domain_power_management_set_ram_suspend_enabled(GVirConfigDomainPowerManagement *pm,
>                                                                  gboolean enabled)
> in other files even if the first line is more than 80 chars

Though it generally is but I wonder which rule to follow in this case.
I usually put more stress on max columns rule than any other because
the implicit assumption in the rule is that N is the size of the
screen/terminal/editor.

>> +{
>> +    g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_POWER_MANAGEMENT(pm));
>> +
>> +    gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(pm),
>> +                                                "suspend-to-ram",
>> +                                                "enabled",
>> +                                                enabled? "yes" : "no");
>
> We can probably add a gvir_config_object_add_child_with_attribute_type at
> some point where you'd pass G_TYPE_BOOLEAN and 'enabled', and this would do
> the transformation for you. We can add that next time this would be useful.

I had the same train of thoughts. :)

>> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-power-management.h b/libvirt-gconfig/libvirt-gconfig-domain-power-management.h
>> new file mode 100644
>> index 0000000..1d9b3b7
>> --- /dev/null
>> +++ b/libvirt-gconfig/libvirt-gconfig-domain-power-management.h
>> @@ -0,0 +1,76 @@
>> +/*
>> + * libvirt-gconfig-domain-power-management.h: libvirt domain power management configuration
>> + *
>> + * Copyright (C) 2012 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
>> + *
>> + * Zeeshan Ali (Khattak) <zeeshanak gnome org>
>> + * Christophe Fergeau <cfergeau redhat com>
>> + */
>> +
>> +#if !defined(__LIBVIRT_GCONFIG_H__) && !defined(LIBVIRT_GCONFIG_BUILD)
>> +#error "Only <libvirt-gconfig/libvirt-gconfig.h> can be included directly."
>> +#endif
>> +
>> +#ifndef __LIBVIRT_GCONFIG_DOMAIN_POWER_MANAGEMENT_H__
>> +#define __LIBVIRT_GCONFIG_DOMAIN_POWER_MANAGEMENT_H__
>> +
>> +#include <libvirt-gconfig/libvirt-gconfig-domain-timer.h>
>
> I don't think we need to include timer?

C&P mistake.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]