[libvirt] [libvirt-designer 0/3] A few more cleanups

Hey, here are a few small patches for libvirt-designer to fix a few leaks/polish minor things. Christophe

--- libvirt-designer/libvirt-designer-domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 4805102..0d47d3c 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -946,7 +946,7 @@ error: * * Add a new disk to the domain. * - * Returns: (transfer none): the pointer to new disk. + * Returns: (transfer full): the pointer to new disk. * If something fails NULL is returned and @error is set. */ GVirConfigDomainDisk *gvir_designer_domain_add_disk_file(GVirDesignerDomain *design, @@ -976,7 +976,7 @@ GVirConfigDomainDisk *gvir_designer_domain_add_disk_file(GVirDesignerDomain *des * * Add given device as a new disk to the domain designer instance. * - * Returns: (transfer none): the pointer to the new disk. + * Returns: (transfer full): the pointer to the new disk. * If something fails NULL is returned and @error is set. */ GVirConfigDomainDisk *gvir_designer_domain_add_disk_device(GVirDesignerDomain *design, @@ -1059,7 +1059,7 @@ cleanup: * 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. + * Returns: (transfer full): the pointer to the new interface. */ GVirConfigDomainInterface * gvir_designer_domain_add_interface_network(GVirDesignerDomain *design, -- 1.8.1.4

On Tue, Apr 02, 2013 at 11:45:09AM +0200, Christophe Fergeau wrote:
--- libvirt-designer/libvirt-designer-domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

We need this to be able to mark the device we are creating as a CDROM. --- examples/virtxml.c | 2 +- libvirt-designer/libvirt-designer-domain.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index d4a5fe2..9a36142 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -712,7 +712,7 @@ Domain with Fedora 17 from locally stored ISO and one NIC with mac To add multiple devices just use appropriate argument multiple times: - # virtxml -d /tmp/Fedora-17-x86_64-Live-KDE.iso,raw \ + # virtxml -d /tmp/Fedora-17-x86_64-Live-KDE.iso,iso \ -d /var/lib/libvirt/images/f17.img,qcow2 \ -i default,mac=00:11:22:33:44:55,link=down \ -i blue_network \ diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 0d47d3c..5da8dd3 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -894,6 +894,14 @@ gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, gvir_config_domain_disk_set_type(disk, type); gvir_config_domain_disk_set_source(disk, path); gvir_config_domain_disk_set_driver_name(disk, driver_name); + if (g_strcmp0(format, "iso") == 0) { + /* FIXME: Should probably reorder the disk devices so that floppies + * go first, then disks, then CDROMs + */ + gvir_config_domain_disk_set_guest_device_type(disk, + GVIR_CONFIG_DOMAIN_DISK_GUEST_DEVICE_CDROM); + format = "raw"; + } if (format) gvir_config_domain_disk_set_driver_type(disk, format); if (g_str_equal(bus_str, "ide")) { -- 1.8.1.4

On Tue, Apr 02, 2013 at 11:45:10AM +0200, Christophe Fergeau wrote:
We need this to be able to mark the device we are creating as a CDROM. --- examples/virtxml.c | 2 +- libvirt-designer/libvirt-designer-domain.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c index d4a5fe2..9a36142 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -712,7 +712,7 @@ Domain with Fedora 17 from locally stored ISO and one NIC with mac
To add multiple devices just use appropriate argument multiple times:
- # virtxml -d /tmp/Fedora-17-x86_64-Live-KDE.iso,raw \ + # virtxml -d /tmp/Fedora-17-x86_64-Live-KDE.iso,iso \ -d /var/lib/libvirt/images/f17.img,qcow2 \ -i default,mac=00:11:22:33:44:55,link=down \ -i blue_network \ diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 0d47d3c..5da8dd3 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -894,6 +894,14 @@ gvir_designer_domain_add_disk_full(GVirDesignerDomain *design, gvir_config_domain_disk_set_type(disk, type); gvir_config_domain_disk_set_source(disk, path); gvir_config_domain_disk_set_driver_name(disk, driver_name); + if (g_strcmp0(format, "iso") == 0) { + /* FIXME: Should probably reorder the disk devices so that floppies + * go first, then disks, then CDROMs + */ + gvir_config_domain_disk_set_guest_device_type(disk, + GVIR_CONFIG_DOMAIN_DISK_GUEST_DEVICE_CDROM); + format = "raw"; + }
I don't think it is a good idea to overload 'format' for this purpose. It is perfectly acceptable to back a CDROM device by a qcow2 files. I think we should just have a gvir_designer_domain_add_cdrom() method or some other indicator Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Apr 02, 2013 at 11:12:29AM +0100, Daniel P. Berrange wrote:
I don't think it is a good idea to overload 'format' for this purpose. It is perfectly acceptable to back a CDROM device by a qcow2 files. I think we should just have a gvir_designer_domain_add_cdrom() method or some other indicator
Now that you point it out, I totally agree. I've went with add_cdrom_file, add_cdrom_device, add_floppy_file, add_floppy_device and sent a v2, though this scheme won't really scale well( it's already missing "lun"). Christophe

--- examples/virtxml.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index 9a36142..1e9f78a 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -178,6 +178,7 @@ add_disk(gpointer data, gpointer user_data) { GVirDesignerDomain *domain = (GVirDesignerDomain *) user_data; + GVirConfigDomainDisk *disk; char *path = (char *) data; char *format = NULL; struct stat buf; @@ -196,10 +197,12 @@ add_disk(gpointer data, if (!stat(path, &buf) && !S_ISREG(buf.st_mode)) { - gvir_designer_domain_add_disk_device(domain, path, &error); + disk = gvir_designer_domain_add_disk_device(domain, path, &error); } else { - gvir_designer_domain_add_disk_file(domain, path, format, &error); + disk = gvir_designer_domain_add_disk_file(domain, path, format, &error); } + if (disk) + g_object_unref(G_OBJECT(disk)); if (error) { print_error("%s", error->message); @@ -493,7 +496,7 @@ main(int argc, char *argv[]) static char *connect_uri = NULL; static char *resources_str = NULL; GVirDesignerDomainResources resources; - GOptionContext *context; + GOptionContext *context = NULL; static GOptionEntry entries[] = { @@ -603,6 +606,8 @@ main(int argc, char *argv[]) ret = EXIT_SUCCESS; cleanup: + if (context) + g_option_context_free(context); if (os) g_object_unref(G_OBJECT(os)); if (platform) -- 1.8.1.4

On Tue, Apr 02, 2013 at 11:45:11AM +0200, Christophe Fergeau wrote:
--- examples/virtxml.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Christophe Fergeau
-
Daniel P. Berrange