Re: [libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support
On Mon, Sep 10, 2012 at 11:38:30AM +0200, Martin Kletzander wrote: > On 09/10/2012 11:19 AM, Christophe Fergeau wrote: > Thanks, but I have to look inside this one, anyway. You confirmed me > what I thought this is what the function is doing, but I saw no need for > cast in here. Now I see it has to be changed from InterfaceNetwork to > Interface. I guess this is some kind of polymorphism/inheritance design > in glib. Yes, C being what it is, you cannot get automatic casts from a child class to one of its parents, so we have to do it explicitly when needed. Christophe pgpXmccY2qWBF.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support
On 09/10/2012 11:19 AM, Christophe Fergeau wrote:
> Hey,
>
> On Fri, Sep 07, 2012 at 03:56:34PM +0200, Martin Kletzander wrote:
>> On 09/05/2012 11:27 AM, Michal Privoznik wrote:
>>> +
>>> +static const gchar *
>>> +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design,
>>> + GError **error)
>>> +{
>>> +const gchar *ret = NULL;
>>> +OsinfoDeviceLink *link = NULL;
>>
>> You are using "link" here which shadows some global declaration. I'm
>> saying "some" because the compiler told me so, but after many files I
>> wasn't so eager to find it, so I don't know which one. Anyway changing
>> it to dev_link (as you use everywhere else) worked.
>
> Probably link(3) which creates hardlinks.
>
Oh, right. I was looking at the code and it starts looking as something
else than C sometimes. Thanks for "waking me up" :)
>>> +
>>> +GVirConfigDomainInterface *ret;
>>> +const gchar *model = NULL;
>>> +
>>> +model = gvir_designer_domain_get_preferred_nic_model(design, error);
>>> +
>>> +ret =
>>> GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
>>
>> I can't find the function anywhere, even though it isn't defined
>> automagically (IIUC), but it compiles (I wonder what the macro does in
>> here).
>
> $prefix/include/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig-interface.h
> has:
> #define GVIR_CONFIG_INTERFACE(obj)(G_TYPE_CHECK_INSTANCE_CAST
> ((obj), GVIR_CONFIG_TYPE_INTERFACE, GVirConfigInterface))
> which is C cast + runtime type check. This is used everywhere in GObject
> land. gvir_config_domain_interface_network_new is defined in
> libvirt-gconfig-domain-interface-network.h
>
Thanks, but I have to look inside this one, anyway. You confirmed me
what I thought this is what the function is doing, but I saw no need for
cast in here. Now I see it has to be changed from InterfaceNetwork to
Interface. I guess this is some kind of polymorphism/inheritance design
in glib.
As I said before, I'm not very familiar with glib (and similar), I just
wanted to check Michal's code and give it some attention :)
Have a nice day,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support
Hey,
On Fri, Sep 07, 2012 at 03:56:34PM +0200, Martin Kletzander wrote:
> On 09/05/2012 11:27 AM, Michal Privoznik wrote:
> > +
> > +static const gchar *
> > +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design,
> > + GError **error)
> > +{
> > +const gchar *ret = NULL;
> > +OsinfoDeviceLink *link = NULL;
>
> You are using "link" here which shadows some global declaration. I'm
> saying "some" because the compiler told me so, but after many files I
> wasn't so eager to find it, so I don't know which one. Anyway changing
> it to dev_link (as you use everywhere else) worked.
Probably link(3) which creates hardlinks.
> > +
> > +GVirConfigDomainInterface *ret;
> > +const gchar *model = NULL;
> > +
> > +model = gvir_designer_domain_get_preferred_nic_model(design, error);
> > +
> > +ret =
> > GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
>
> I can't find the function anywhere, even though it isn't defined
> automagically (IIUC), but it compiles (I wonder what the macro does in
> here).
$prefix/include/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig-interface.h
has:
#define GVIR_CONFIG_INTERFACE(obj)(G_TYPE_CHECK_INSTANCE_CAST
((obj), GVIR_CONFIG_TYPE_INTERFACE, GVirConfigInterface))
which is C cast + runtime type check. This is used everywhere in GObject
land. gvir_config_domain_interface_network_new is defined in
libvirt-gconfig-domain-interface-network.h
Christophe
pgpv7KIbiUrKG.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support
On 09/05/2012 11:27 AM, Michal Privoznik wrote:
> Let users add NICs to domains.
> ---
> examples/virtxml.c | 96
>
> libvirt-designer/libvirt-designer-domain.c | 53 +++
> libvirt-designer/libvirt-designer-domain.h |3 +
> libvirt-designer/libvirt-designer.sym |1 +
> 4 files changed, 141 insertions(+), 12 deletions(-)
>
[...]
> +
> +static const gchar *
> +gvir_designer_domain_get_preferred_nic_model(GVirDesignerDomain *design,
> + GError **error)
> +{
> +const gchar *ret = NULL;
> +OsinfoDeviceLink *link = NULL;
You are using "link" here which shadows some global declaration. I'm
saying "some" because the compiler told me so, but after many files I
wasn't so eager to find it, so I don't know which one. Anyway changing
it to dev_link (as you use everywhere else) worked.
> +
> +link = gvir_designer_domain_get_preferred_device(design, "network",
> error);
> +if (!link)
> +goto cleanup;
> +
> +ret = osinfo_devicelink_get_driver(link);
> +
> +cleanup:
> +if (link)
> +g_object_unref(link);
> +return ret;
> +}
> +
> +/**
> + * gvir_designer_domain_add_interface_network:
> + * @design: (transfer none): the domain designer instance
> + * @network: (transfer none): network name
> + *
> + * Add new network interface card into @design. The interface is
> + * of 'network' type with @network used as the source network.
> + *
> + * Returns: (transfer none): the pointer to the new interface.
> + */
> +GVirConfigDomainInterface *
> +gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
> + const char *network,
> + GError **error)
> +{
Are you planning on using gvir...add_interface_full with
add_interface_{network,bridge,etc.} as "wrappers"? I liked that with the
disk in the first patch.
> +g_return_val_if_fail(GVIR_DESIGNER_IS_DOMAIN(design), NULL);
You check this value here but not on all the previous places (and patches).
> +
> +GVirConfigDomainInterface *ret;
> +const gchar *model = NULL;
> +
> +model = gvir_designer_domain_get_preferred_nic_model(design, error);
> +
> +ret =
> GVIR_CONFIG_DOMAIN_INTERFACE(gvir_config_domain_interface_network_new());
I can't find the function anywhere, even though it isn't defined
automagically (IIUC), but it compiles (I wonder what the macro does in
here).
> +
> +
> gvir_config_domain_interface_network_set_source(GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(ret),
> +network);
> +if (model)
> +gvir_config_domain_interface_set_model(ret, model);
> +
> +gvir_config_domain_add_device(design->priv->config,
> GVIR_CONFIG_DOMAIN_DEVICE(ret));
> +
> +return ret;
> +}
> diff --git a/libvirt-designer/libvirt-designer-domain.h
> b/libvirt-designer/libvirt-designer-domain.h
> index 06a5749..5097393 100644
> --- a/libvirt-designer/libvirt-designer-domain.h
> +++ b/libvirt-designer/libvirt-designer-domain.h
> @@ -101,6 +101,9 @@ GVirConfigDomainDisk
> *gvir_designer_domain_add_disk_device(GVirDesignerDomain *d
> const char
> *devpath,
> GError **error);
>
> +GVirConfigDomainInterface
> *gvir_designer_domain_add_interface_network(GVirDesignerDomain *design,
> + const
> char *network,
> + GError
> **error);
> G_END_DECLS
>
> #endif /* __LIBVIRT_DESIGNER_DOMAIN_H__ */
> diff --git a/libvirt-designer/libvirt-designer.sym
> b/libvirt-designer/libvirt-designer.sym
> index e67323a..77f76b4 100644
> --- a/libvirt-designer/libvirt-designer.sym
> +++ b/libvirt-designer/libvirt-designer.sym
> @@ -12,6 +12,7 @@ LIBVIRT_DESIGNER_0.0.1 {
>
> gvir_designer_domain_add_disk_file;
> gvir_designer_domain_add_disk_device;
> +gvir_designer_domain_add_interface_network;
>
Indentation.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support
Let users add NICs to domains.
---
examples/virtxml.c | 96
libvirt-designer/libvirt-designer-domain.c | 53 +++
libvirt-designer/libvirt-designer-domain.h |3 +
libvirt-designer/libvirt-designer.sym |1 +
4 files changed, 141 insertions(+), 12 deletions(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c
index 36bc3bb..87929b2 100644
--- a/examples/virtxml.c
+++ b/examples/virtxml.c
@@ -56,17 +56,19 @@ print_usage(const char *progname)
{
printf("\nUsage: %s options ...\n"
" options:\n"
- " -h | --help print this help\n"
- " -c | --connect=URI libvirt connection URI used \n"
- " for querying capabilities\n"
- " --list-os list IDs of known OSes\n"
- " --list-platformlist IDs of known hypervisors\n"
- " -o | --os=OSset domain OS\n"
- " -p | --platform=PLATFORMset hypervisor under which \n"
- " domain will be running\n"
- " -a | --architecture=ARCHset domain architecture\n"
- " -d | --disk=PATH[,FORMAT] add disk to domain with PATH being
\n"
- " source and FORMAT is its format\n",
+ " -h | --help print this help\n"
+ " -c | --connect=URIlibvirt connection URI
used \n"
+ "for querying
capabilities\n"
+ " --list-oslist IDs of known OSes\n"
+ " --list-platform list IDs of known
hypervisors\n"
+ " -o | --os=OS set domain OS\n"
+ " -p | --platform=PLATFORM set hypervisor under which
\n"
+ "domain will be running\n"
+ " -a | --architecture=ARCH set domain architecture\n"
+ " -d | --disk=PATH[,FORMAT] add disk to domain with
PATH being \n"
+ "source and FORMAT is its
format\n"
+ " -i | --interface=NETWORK[,ARG=VAL]add interface with NETWORK
source.\n"
+ "Possible ARGs:
mac,link={up,down}\n",
progname);
}
@@ -186,6 +188,69 @@ add_disk(gpointer data,
}
+static void
+add_iface(gpointer data,
+ gpointer user_data)
+{
+GVirDesignerDomain *domain = (GVirDesignerDomain *) user_data;
+char *network = (char *) data;
+char *param = NULL;
+GVirConfigDomainInterface *iface = NULL;
+GError *error = NULL;
+
+param = strchr(network, ',');
+if (param) {
+*param = '\0';
+param++;
+}
+
+iface = gvir_designer_domain_add_interface_network(domain, network,
&error);
+if (error) {
+print_error("%s", error->message);
+exit(EXIT_FAILURE);
+}
+
+while (param && *param) {
+char *key = param;
+char *val;
+GVirConfigDomainInterfaceLinkState link;
+
+/* move to next token */
+param = strchr(param, ',');
+if (param) {
+*param = '\0';
+param++;
+}
+
+/* parse token */
+val = strchr(key, '=');
+if (!val) {
+print_error("Invalid format: %s", key);
+exit(EXIT_FAILURE);
+}
+
+*val = '\0';
+val++;
+
+if (!strcmp(key, "mac")) {
+gvir_config_domain_interface_set_mac(iface, val);
+} else if (!strcmp(key, "link")) {
+if (!strcmp(val, "up")) {
+link = GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_UP;
+} else if (!strcmp(val, "down")) {
+link = GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DOWN;
+} else {
+print_error("Unknown value: %s", val);
+exit(EXIT_FAILURE);
+}
+gvir_config_domain_interface_set_link_state(iface, link);
+} else {
+print_error("Unknown key: %s", key);
+exit(EXIT_FAILURE);
+}
+}
+}
+
#define CHECK_ERROR \
if (error) {\
print_error("%s", error->message); \
@@ -210,6 +275,7 @@ main(int argc, char *argv[])
char *arch_str = NULL;
char *connect_uri = NULL;
GList *disk_str_list = NULL;
+GList *iface_str_list = NULL;
int arg;
struct option opt[] = {
@@ -221,6 +287,7 @@ main(int argc, char *argv[])
{"platform", required_argument, NULL, 'p'},
{"architecture", required_argument, NULL, 'a'},
{"disk", required_argument, NULL, 'd'},
+{"interface", required_argument, NULL, 'i'},
{NULL, 0, NULL, 0}
};
@@ -230,7 +297,7 @@ main(int argc, char *a
