[libvirt] [PATCH] remove unnecessary "V = NULL; " stmts after VIR_FREE(V)
by Jim Meyering
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
16 years, 3 months
[libvirt] Libvirtd default.xml (in qemu/networks/autostart) breaks XEN
by Stefan de Konink
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
16 years, 3 months
[libvirt] virStorageVolCreateXML vs virStorageVolCloneXML
by Stefan de Konink
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
16 years, 3 months
[libvirt] [PATCH]: Add floppy support to xm_internal
by Chris Lalancette
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>
16 years, 3 months
[libvirt] RFC? finding potential storage pool resources
by David Lively
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
16 years, 3 months
[libvirt] attach device on a defined (non-active) vm
by Stefan de Konink
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
16 years, 3 months
[libvirt] [PATCH] enable format-safety checks for virDomainReportError
by Jim Meyering
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
16 years, 3 months
[libvirt] [PATCH] Fix pool-create for netfs format 'auto'
by Cole Robinson
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);
16 years, 3 months
[libvirt] some unsorted questions
by Stefan de Konink
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
16 years, 3 months
