- Devel - libvirt List Archives

[libvirt] [PATCH] remove unnecessary "V = NULL; " stmts after VIR_FREE(V)
by Jim Meyering 19 Jul '08

19 Jul '08

Doing a review (in progress), I spotted one of these, so went in search of others. They're harmless, so this is more a heads up than anything else. I am happy to defer application until the patch queue has been reduced. >From d97af7667699529c216835d806d1f0c6f698a70d Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 17 Jul 2008 12:05:44 +0200 Subject: [PATCH] remove unnecessary "V = NULL;" stmts after VIR_FREE(V) * src/domain_conf.c (virDomainChrDefParseXML) (virDomainNetDefParseXML): Likewise. * src/iptables.c (iptRuleFree): Likewise. * src/storage_backend.c (virStorageBackendRunProgRegex): Likewise. * src/test.c (testOpenFromFile): Likewise. * src/xm_internal.c (xenXMAttachInterface): Likewise. * src/xml.c (virDomainParseXMLOSDescHVM): Likewise. * src/xmlrpc.c (xmlRpcCallRaw): Likewise. --- src/domain_conf.c | 5 +---- src/iptables.c | 2 -- src/storage_backend.c | 4 +--- src/test.c | 8 ++------ src/xm_internal.c | 1 - src/xml.c | 1 - src/xmlrpc.c | 1 - 7 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 82c0ee6..6340c2a 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -696,7 +696,6 @@ virDomainNetDefParseXML(virConnectPtr conn, if (STRPREFIX((const char*)ifname, "vnet")) { /* An auto-generated target name, blank it out */ VIR_FREE(ifname); - ifname = NULL; } } else if ((script == NULL) && (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET) && @@ -954,10 +953,8 @@ virDomainChrDefParseXML(virConnectPtr conn, bindService = virXMLPropString(cur, "service"); } - if (def->type == VIR_DOMAIN_CHR_TYPE_UDP) { + if (def->type == VIR_DOMAIN_CHR_TYPE_UDP) VIR_FREE(mode); - mode = NULL; - } } } else if (xmlStrEqual(cur->name, BAD_CAST "protocol")) { if (protocol == NULL) diff --git a/src/iptables.c b/src/iptables.c index e7613e2..3e3a1a2 100644 --- a/src/iptables.c +++ b/src/iptables.c @@ -266,14 +266,12 @@ static void iptRuleFree(iptRule *rule) { VIR_FREE(rule->rule); - rule->rule = NULL; if (rule->argv) { int i = 0; while (rule->argv[i]) VIR_FREE(rule->argv[i++]); VIR_FREE(rule->argv); - rule->argv = NULL; } } diff --git a/src/storage_backend.c b/src/storage_backend.c index 3e4e39c..a164a08 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -444,10 +444,8 @@ virStorageBackendRunProgRegex(virConnectPtr conn, goto cleanup; /* Release matches & restart to matching the first regex */ - for (j = 0 ; j < totgroups ; j++) { + for (j = 0 ; j < totgroups ; j++) VIR_FREE(groups[j]); - groups[j] = NULL; - } maxReg = 0; ngroup = 0; } diff --git a/src/test.c b/src/test.c index d0bb003..b7b9df0 100644 --- a/src/test.c +++ b/src/test.c @@ -461,10 +461,8 @@ static int testOpenFromFile(virConnectPtr conn, dom->def->id = privconn->nextDomID++; dom->persistent = 1; } - if (domains != NULL) { + if (domains != NULL) VIR_FREE(domains); - domains = NULL; - } ret = virXPathNodeSet("/node/network", ctxt, &networks); if (ret < 0) { @@ -498,10 +496,8 @@ static int testOpenFromFile(virConnectPtr conn, net->persistent = 1; } - if (networks != NULL) { + if (networks != NULL) VIR_FREE(networks); - networks = NULL; - } xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); diff --git a/src/xm_internal.c b/src/xm_internal.c index 3b264a8..60f32fe 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2925,7 +2925,6 @@ xenXMAttachInterface(virDomainPtr domain, xmlXPathContextPtr ctxt, int hvm, if (virMacAddrCompare (dommac, (const char *) mac) == 0) { if (autoassign) { VIR_FREE(mac); - mac = NULL; if (!(mac = (xmlChar *)xenXMAutoAssignMac())) goto cleanup; /* initialize the list */ diff --git a/src/xml.c b/src/xml.c index d5730ed..477d466 100644 --- a/src/xml.c +++ b/src/xml.c @@ -1200,7 +1200,6 @@ virDomainParseXMLOSDescHVM(virConnectPtr conn, xmlNodePtr node, xmlFree(bus); } VIR_FREE(nodes); - nodes = NULL; } cur = virXPathNode("/domain/devices/parallel[1]", ctxt); diff --git a/src/xmlrpc.c b/src/xmlrpc.c index d627607..cbca389 100644 --- a/src/xmlrpc.c +++ b/src/xmlrpc.c @@ -443,7 +443,6 @@ static char *xmlRpcCallRaw(const char *url, const char *request) if (ret != len) { errno = EINVAL; VIR_FREE(response); - response = NULL; xmlRpcError(VIR_ERR_POST_FAILED, _("read response"), 0); } -- 1.5.6.3.385.g9f9af

2 2

[libvirt] Libvirtd default.xml (in qemu/networks/autostart) breaks XEN
by Stefan de Konink 18 Jul '08

18 Jul '08

The desire to automatically install the autostarted network configuration of libvirt broke my (and some other users on xen-users) setup. I suggest to *remove* this network configuration as default and *not* put it into xenstore as a stateful config. *It does not work by default* Worse is that it is almost impossible to debug if you don't know where to find it. Stefan

2 5

[libvirt] virStorageVolCreateXML vs virStorageVolCloneXML
by Stefan de Konink 18 Jul '08

18 Jul '08

Hi, There is currently no implementation in the api to clone snapshots or images. I wonder if we could add an XML node to specify a backed device. Or add a new function that allows to clone. Like the create is not supported by all pools, cloning should also be based on best effort, falling back to cp for non sparse images. (Probably a good configuration option) Now the new XML parser is in place I have still the desire to create this type of configuration: <disk type='pool'> <source pool='netapp' volume='lun-3' /> <target dev='xvda'/> </disk> And it might be a useful source tag for cloning too. Stefan

2 2

[libvirt] [PATCH]: Add floppy support to xm_internal
by Chris Lalancette 18 Jul '08

18 Jul '08

Yes, you read the subject right; add floppy support to xm internal. Let's just say I didn't do this by choice. In any case, there was a cryptic comment in xenXMParseXMLDisks() that said: /* Xend (all versions) put the floppy device config * under the hvm (image (os)) block */ What this actually means is that we shouldn't parse the floppy stuff to put it in the "disks =" section of the /etc/xen configuration file, since it doesn't have meaning there. Instead, floppy disks go at the top-level of a Xen config file, like: fda = '/var/lib/xen/images/floppy.img' fdb = '/var/lib/xen/images/floppy2.img' That's exactly what this patch does. In combination with a couple of other small patches to virt-install (which I will post to the appropriate list), I was able to use a floppy disk to hold the kickstart for a fully virtualized Xen guest install. Signed-off-by: Chris Lalancette <clalance(a)redhat.com>

3 4

[libvirt] RFC? finding potential storage pool resources
by David Lively 18 Jul '08

18 Jul '08

Hi - I'm looking into using (which I think means extending) libvirt to enumerate "potential" storage pool resources, in particular: * existing physical disk device names (for creating "disk" pools) * existing logical volume group names (for creating "logical" pools) Note that List{Defined,Active}StorageGroups don't do the trick. Suppose this is a new host and I'm trying to start defining the storage pools (and I want to be able to use existing volume groups, for example). I don't see how to do that within the current libvirt framework. If I'm missing something, please let me know (and ignore the rest of this message ...). This could be done by adding some new calls like: int virConnectListPhysDisks(virConnectPtr conn, char ** const name, int maxnames) int virConnectListLogicalVolGroups(virConnectPtr conn, char ** const name, int maxnames) ... plus a pair of NumOf functions ... But these are each storage-driver specific. For example, if I'm not using the "logical" storage driver, I have no need (or means) of listing volume groups. So maybe it's cleaner to fold these two functions into one, now parameterized by storage driver type: int virConnectListStorageSources(virConnectPtr conn, const char *type, char ** const name, int maxnames) ... plus a NumOf function ... where <type> is one of the supported storage pool types. So, if <type> is "disk", ListStorageSources acts like ListPhysDisks, and if <type> is "logical", ListStorageSources acts like ListLogicalVolumeGroups, (and we return empty lists or some sort of "unsupported" error for any other types ... can't list all possible network servers, for instance). What do you all think? Thanks, Dave

2 2

[libvirt] attach device on a defined (non-active) vm
by Stefan de Konink 18 Jul '08

18 Jul '08

It seems that the attach device function really 'attaches' a (live) device to a non-active domain. Is this by design? Otherwise I would not have a clue how to interpreted this message: libvir: Xen Daemon error : POST operation failed: (xend.err 'Device /dev/xvdp (51952, tap) is already connected.') (when I try to start the domain) So what I did; I have created an empty domain without any devices. Loaded that and added a disk. Stefan

2 1

[libvirt] [PATCH] enable format-safety checks for virDomainReportError
by Jim Meyering 18 Jul '08

18 Jul '08

This enables format-safety checks for virDomainReportError, so that if you try to print e.g., an int via "%s", gcc will detect it. The Makefile.maint check ensures that all string arguments to virDomainReportError are marked for translation with _(...). >From 65c7fc8bdbf491a02d1d14fcdef429ba1c9e0dea Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 18 Jul 2008 12:46:38 +0200 Subject: [PATCH] enable format-safety checks for virDomainReportError * src/domain_conf.c (virDomainReportError): Declare using ATTRIBUTE_FORMAT(printf, 3, 4). * Makefile.maint (msg_gen_function): Add virDomainReportError. --- Makefile.maint | 1 + src/domain_conf.c | 4 ++++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/Makefile.maint b/Makefile.maint index 5da2984..03800f8 100644 --- a/Makefile.maint +++ b/Makefile.maint @@ -352,6 +352,7 @@ msg_gen_function += ReportError msg_gen_function += qemudReportError msg_gen_function += openvzLog msg_gen_function += openvzError +msg_gen_function += virDomainReportError # Uncomment the following and run "make syntax-check" to see diagnostics # that are not yet marked for translation, but that need to be rewritten diff --git a/src/domain_conf.c b/src/domain_conf.c index 82c0ee6..2ff5d1a 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -128,6 +128,10 @@ VIR_ENUM_IMPL(virDomainGraphics, VIR_DOMAIN_GRAPHICS_TYPE_LAST, static void virDomainReportError(virConnectPtr conn, int code, const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 3, 4); + +static void virDomainReportError(virConnectPtr conn, + int code, const char *fmt, ...) { va_list args; char errorMessage[1024]; -- 1.5.6.3.440.ge3a8d

2 2

[libvirt] [PATCH]: Add MIGRATE_LIVE definition to ruby-libvirt bindings
by Chris Lalancette 18 Jul '08

18 Jul '08

Attached is a trivial patch to add the MIGRATE_LIVE flag into the ruby-libvirt bindings. Signed-off-by: Chris Lalancette <clalance(a)redhat.com>

2 2

[libvirt] [PATCH] Fix pool-create for netfs format 'auto'
by Cole Robinson 17 Jul '08

17 Jul '08

Trying to pool-create a netfs pool with the format type 'auto' (as in, to autodetect the format) runs the command mount -t auto munged-source-path '-t auto' seems to do its job for regular file systems, but actually fails with nfs or cifs (I assume anything that required an external mount program). Strangely though, the command mount munged-source-path will do the right thing. The attached patch fixes the generated command to work in the above case, fully removing the '-t type' piece if auto is specified for a netfs pool. I tested the intended case, as well as regular fs pools format=auto, and netfs format=nfs, and all seemed to work fine. Thanks, Cole diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 9926493..f195034 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -487,7 +487,23 @@ static int virStorageBackendFileSystemMount(virConnectPtr conn, virStoragePoolObjPtr pool) { char *src; - const char *mntargv[] = { + const char **mntargv; + + /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), + * while plain 'mount' does. We have to craft separate argvs to + * accommodate this */ + int netauto = (pool->def->type == VIR_STORAGE_POOL_NETFS && + pool->def->source.format == VIR_STORAGE_POOL_NETFS_AUTO); + int source_index; + + const char *netfs_auto_argv[] = { + MOUNT, + NULL, /* source path */ + pool->def->target.path, + NULL, + }; + + const char *fs_argv[] = { MOUNT, "-t", pool->def->type == VIR_STORAGE_POOL_FS ? @@ -495,10 +511,20 @@ virStorageBackendFileSystemMount(virConnectPtr conn, pool->def->source.format) : virStorageBackendFileSystemNetPoolFormatToString(conn, pool->def->source.format), - NULL, /* Fill in shortly - careful not to add extra fields before this */ + NULL, /* Fill in shortly - careful not to add extra fields + before this */ pool->def->target.path, NULL, }; + + if (netauto) { + mntargv = netfs_auto_argv; + source_index = 1; + } else { + mntargv = fs_argv; + source_index = 3; + } + int ret; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { @@ -543,7 +569,7 @@ virStorageBackendFileSystemMount(virConnectPtr conn, return -1; } } - mntargv[3] = src; + mntargv[source_index] = src; if (virRun(conn, (char**)mntargv, NULL) < 0) { VIR_FREE(src);

4 3

[libvirt] some unsorted questions
by Stefan de Konink 17 Jul '08

17 Jul '08

Hi, Currently I'm working on my webserver thingie. And I notice some annoying error messages. Generally I use a predefined host to do the basic setup of defining a domain. So a client can shoot its XML directly into a post request, or we set it up by parsing some parameters from the URI. Now personally I think it is smart to check if a domain is already defined, or in use. If this is not the case libvirtd and the client get this message: libvir: Xen Daemon error : GET operation failed: That doesn't seem to fit the scenario at all :) Why does lookupbyname behave so bad? Secondly; I am a bit distracted by the domids concept. These ids are not available before a domain is launched. I think it would be interesting to allow signed values. In this way the 'defined' not active domains would get a negative value and a running domain a positive value. (Dom0 gets 0) This would have far less implications than using an uuid through the codebase consistently (not speaking the about the extra overhead). If this gets implemented I would suggest a call that fetches all domains from a running system and not only the defined or only the active ones. Stefan

4 6