- Devel - libvirt List Archives

Re: [libvirt] [PATCH 02/10] Add VIR_SECRET_GET_VALUE_INTERNAL_CALL.
by Miloslav Trmac 08 Sep '09

08 Sep '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > > +/* Make sure ... INTERNAL_CALL can not be set by the caller */ > > +verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & > > + VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); > > ??? what's that ? an assert at compile time ? I don't know that construct > I would rather avoid it if it's not portable. Yes, a compile-time assert, provided by gnulib. The mechanism depends only on ISO C, and gnulib/lib/verify.h contains detailed comments about why the particular variant was chosen. It seems safe enough to me. Mirek

1 0

[libvirt] [PATCH]: Fixed minor bugs in display and added OOM checks.
by Pritesh Kothari 08 Sep '09

08 Sep '09

Hi All, There was a minor bug in selecting the graphics type. if the graphics type was desktop it was assumed that display is set for it, and thus crashed on strdup, so now checking if display is present before setting it. The second bug was while setting the 3d acceleration parameter, VIR_ALLOC() returns 0 or greater if success and not the other way round. Lastly added OOM checks in few places which were missed earlier. Regards, Pritesh

3 4

[libvirt] [PATCH] 6 dead-store patches
by Jim Meyering 07 Sep '09

07 Sep '09

>From 129dc57243df4e73daa24aac671087bcd25f51ad Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 16:49:37 +0200 Subject: [PATCH 1/6] xm_internal.c: remove dead stores of local, "type" * src/xm_internal.c (xenXMDomainConfigParse): Remove declaration and useless containing if-block, too. --- src/xm_internal.c | 9 +-------- 1 files changed, 1 insertions(+), 8 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 9627ffb..661172b 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007 Red Hat + * Copyright (C) 2006-2007, 2009 Red Hat * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -995,7 +995,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if (list && list->type == VIR_CONF_LIST) { list = list->list; while (list) { - int type = -1; char script[PATH_MAX]; char model[10]; char ip[16]; @@ -1031,7 +1030,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { mac[len] = '\0'; } else if (STRPREFIX(key, "bridge=")) { int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; - type = 1; if (len > (sizeof(bridge)-1)) len = sizeof(bridge)-1; strncpy(bridge, data, len); @@ -1069,11 +1067,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { key = nextkey; } - /* XXX Forcing to pretend its a bridge */ - if (type == -1) { - type = 1; - } - if (VIR_ALLOC(net) < 0) goto cleanup; -- 1.6.4.2.419.gab238 >From b8b1ea4894ac9bf2a59472958a8bb0749526847f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:22:19 +0200 Subject: [PATCH 2/6] xm_internal.c: remove two ret=... dead stores * src/xm_internal.c (xenXMDomainCreate): Remove dead stores. --- src/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 661172b..de3aca9 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1850,10 +1850,10 @@ int xenXMDomainCreate(virDomainPtr domain) { goto error; domain->id = ret; - if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0) + if (xend_wait_for_devices(domain->conn, domain->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(domain)) < 0) + if (xenDaemonDomainResume(domain) < 0) goto error; xenUnifiedUnlock(priv); -- 1.6.4.2.419.gab238 >From a74d747d27bde38ece90df9a14acb648d83b9993 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH 3/6] domain_conf.c: remove two dead stores * src/domain_conf.c (virDomainSaveXML): Remove use and decl of "err". (virDomainDefParseXML): Likewise. --- src/domain_conf.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..050cf50 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -2520,8 +2520,7 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, /* Extract domain uuid */ tmp = virXPathString(conn, "string(./uuid[1])", ctxt); if (!tmp) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { + if (virUUIDGenerate(def->uuid)) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); goto error; @@ -4456,12 +4455,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.419.gab238 >From b9c2d697d732b4c740fd9e1f87ad135c52e02c34 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 18:53:20 +0200 Subject: [PATCH 4/6] util.c: avoid dead store to "flag" * src/util.c (virExecDaemonize): Change flag |= VAR to "flag | VAR". --- src/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util.c b/src/util.c index 2529837..af50028 100644 --- a/src/util.c +++ b/src/util.c @@ -663,7 +663,7 @@ int virExecDaemonize(virConnectPtr conn, ret = virExecWithHook(conn, argv, envp, keepfd, retpid, infd, outfd, errfd, - flags |= VIR_EXEC_DAEMON, + flags | VIR_EXEC_DAEMON, hook, data, pidfile); /* __virExec should have set an error */ -- 1.6.4.2.419.gab238 >From 6470cb9312a13fd018fb83b79e66cb50190b1f4f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:36:17 +0200 Subject: [PATCH 5/6] iptables.c: remove dead store to "s" * src/iptables.c (iptablesAddRemoveRule): Remove dead store. --- src/iptables.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/iptables.c b/src/iptables.c index 73b39d1..4562800 100644 --- a/src/iptables.c +++ b/src/iptables.c @@ -398,7 +398,7 @@ iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...) 1; /* arg */ va_start(args, arg); - while ((s = va_arg(args, const char *))) + while (va_arg(args, const char *)) n++; va_end(args); -- 1.6.4.2.419.gab238 >From 65c10b030fb43d6678be9bd5eeeb326a024d6902 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:37:37 +0200 Subject: [PATCH 6/6] network_driver.c: remove dead store to "err" * src/network_driver.c (networkSetAutostart): ...and its decl. --- src/network_driver.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/src/network_driver.c b/src/network_driver.c index 84910ab..49855bf 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -1428,9 +1428,7 @@ static int networkSetAutostart(virNetworkPtr net, goto cleanup; if (autostart) { - int err; - - if ((err = virFileMakePath(driver->networkAutostartDir))) { + if (virFileMakePath(driver->networkAutostartDir)) { virReportSystemError(net->conn, errno, _("cannot create autostart directory '%s'"), driver->networkAutostartDir); -- 1.6.4.2.419.gab238

2 3

[libvirt] [PATCH] xm_internal.c: remove dead store and associated leak
by Jim Meyering 07 Sep '09

07 Sep '09

This one took a little thinking. I'm not 100% sure that the comment in my log is sufficient justification for removing the virGetDomain call. If we leave it, we'll have to free it and remove the XXX comment. >From 324e07f17e6a2ed6df236af955adc693670d1b16 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:46:59 +0200 Subject: [PATCH] xm_internal.c: remove dead store and associated leak * src/xm_internal.c (xenXMDomainDefineXML): Dead store to "olddomain" -- and comment ;-) highlighted that the virGetDomain call represented a leak. It was also useless, seeing as how preceding call to virHashLookup(priv->nameConfigMap found def->name. --- src/xm_internal.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index de3aca9..e7f6a55 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2532,7 +2532,6 @@ cleanup: */ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { virDomainPtr ret; - virDomainPtr olddomain; char filename[PATH_MAX]; const char * oldfilename; virDomainDefPtr def = NULL; @@ -2578,10 +2577,6 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { goto error; } - /* XXX wtf.com is this line for - it appears to be amemory leak */ - if (!(olddomain = virGetDomain(conn, def->name, entry->def->uuid))) - goto error; - /* Remove the name -> filename mapping */ if (virHashRemoveEntry(priv->nameConfigMap, def->name, NULL) < 0) { xenXMError(conn, VIR_ERR_INTERNAL_ERROR, -- 1.6.4.2.419.gab238

2 1

[libvirt] [PATCH] network_conf.c: remove dead store to "err"
by Jim Meyering 07 Sep '09

07 Sep '09

>From 66133e3b9d80e9fadefd810786b0ff2f5c8e6875 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:15:51 +0200 Subject: [PATCH] network_conf.c: remove dead store to "err" * src/network_conf.c (virNetworkDefParseXML): ...and its decl. --- src/network_conf.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/network_conf.c b/src/network_conf.c index 58a4f32..3764bb4 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2009 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -329,8 +329,7 @@ virNetworkDefParseXML(virConnectPtr conn, /* Extract network uuid */ tmp = virXPathString(conn, "string(./uuid[1])", ctxt); if (!tmp) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { + if (virUUIDGenerate(def->uuid)) { virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); goto error; -- 1.6.4.2.409.g85dc3

2 1

[libvirt] merge-commit push prohibition now in place, for master
by Jim Meyering 07 Sep '09

07 Sep '09

Just today we noticed that it was possible to push accidental merge commits to "master". While the commit hook to prevent that was in place, the config setting to enable the hook for "master" was missing. I've just done this on the server: git --git-dir /git/libvirt.git config hooks.denymerge.master true so *now*, we should be protected from ourselves.

2 1

[libvirt] [PATCH] xend_internal.c: Remove two dead stores to "ret"
by Jim Meyering 07 Sep '09

07 Sep '09

>From 99610dd77b95914623bd78f2bcac5d15a6c87024 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:14:09 +0200 Subject: [PATCH] xend_internal.c: Remove two dead stores to "ret" * src/xend_internal.c (xenDaemonCreateXML): Don't set "ret" after last use. --- src/xend_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index cf45cd6..2fa08f1 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -4028,10 +4028,10 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, if (!(dom = virDomainLookupByName(conn, def->name))) goto error; - if ((ret = xend_wait_for_devices(conn, def->name)) < 0) + if (xend_wait_for_devices(conn, def->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(dom)) < 0) + if (xenDaemonDomainResume(dom) < 0) goto error; virDomainDefFree(def); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] openvz_driver.c: avoid dead store to "err"
by Jim Meyering 07 Sep '09

07 Sep '09

>From 559c1ba31a89fd3a6ed54631d5d900fa16140187 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 19:11:31 +0200 Subject: [PATCH] openvz_driver.c: avoid dead store to "err" * src/openvz_driver.c (openvzGenerateContainerVethName): Remove use and decl of "err". --- src/openvz_driver.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/openvz_driver.c b/src/openvz_driver.c index 43d54f9..a8c24ba 100644 --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -504,11 +504,10 @@ openvzGenerateVethName(int veid, char *dev_name_ve) static char * openvzGenerateContainerVethName(int veid) { - int ret; char temp[1024]; /* try to get line "^NETIF=..." from config */ - if ( (ret = openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp))) <= 0) { + if (openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp)) <= 0) { snprintf(temp, sizeof(temp), "eth0"); } else { char *saveptr; -- 1.6.4.2.409.g85dc3

2 1

[libvirt] 3 more dead stores
by Jim Meyering 07 Sep '09

07 Sep '09

>From 7a2202539b6445017c420644e5327ce64eaf3bac Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH 1/2] domain_conf.c: remove two dead stores * src/domain_conf.c (virDomainSaveXML): Remove use and decl of "err". (virDomainDefParseXML): Likewise. --- src/domain_conf.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..050cf50 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -2520,8 +2520,7 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, /* Extract domain uuid */ tmp = virXPathString(conn, "string(./uuid[1])", ctxt); if (!tmp) { - int err; - if ((err = virUUIDGenerate(def->uuid))) { + if (virUUIDGenerate(def->uuid)) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate UUID")); goto error; @@ -4456,12 +4455,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.409.g85dc3 >From 89e2837b4382690e4d434c086541da1dcd3d4c58 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 18:53:20 +0200 Subject: [PATCH 2/2] util.c: avoid dead store to "flag" * src/util.c (virExecDaemonize): Change flag |= VAR to "flag | VAR". --- src/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util.c b/src/util.c index 2529837..af50028 100644 --- a/src/util.c +++ b/src/util.c @@ -663,7 +663,7 @@ int virExecDaemonize(virConnectPtr conn, ret = virExecWithHook(conn, argv, envp, keepfd, retpid, infd, outfd, errfd, - flags |= VIR_EXEC_DAEMON, + flags | VIR_EXEC_DAEMON, hook, data, pidfile); /* __virExec should have set an error */ -- 1.6.4.2.409.g85dc3

2 1

Re: [libvirt] [PATCH 0/6] sVirt AppArmor security driver
by Daniel Veillard 07 Sep '09

07 Sep '09

[ Sending again as my mail Sat, Sep 05, 2009 at 11:00:46AM +0200 didn't seem to have reach the list, Daniel ] On Fri, Sep 04, 2009 at 04:03:50PM -0500, Jamie Strandboge wrote: > On Fri, 04 Sep 2009, Daniel Veillard wrote: > > > On Fri, Sep 04, 2009 at 12:23:33PM -0500, Jamie Strandboge wrote: > > > This patch series implements the AppArmor security driver for sVirt. > > > This implementation was developed for the Ubuntu AppArmorLibvirtProfile > > > specification[1], but is general enough for any AppArmor deployment > > > (such as Ubuntu, *SUSE and Mandriva). > > > > > > This patch has seen quite a bit of real world testing in Ubuntu 9.10 > > > (our development release) in our 0.7.0-1ubuntu3 package. I did make a > > > few small changes after going through HACKING, but mostly I got the > > > tests going and added documentation. > > > > Could you triple check that the make syntax-check run clean with > > your patches applied, because a very quick glimpse shows up a number > > of malloc() which we don't allow for example. It may be because you > > didn't enter the files in the SCM but the checks should be done anyway > > on the new files please. > > > > I noticed that syntax-check wouldn't scan the new files unless I did a > git commit first. I did not run 'syntax-check' after adding the > documention examples, and just found that examples/apparmor/libvirt-qemu > had a trailing blank line (I was a bit surprised syntax-check checked > these files (attached is an updated docs patch)). okay > I can demonstrate that the files are being checked by removing the > '#include <config.h>' line in security_apparmor.c and virt-aa-helper.c > and syntax-check failing: [...] > I don't see that 'syntax-check' enforces not using *alloc, but I did > read in HACKING that *alloc are deprecated, though I had already written > the code before reading it. I do see other examples of *alloc in the I really though all cases of direct malloc or realloc had been eliminated, apparently it's not the case yet, and syntax-check doesn't enforce it... we need to do this. I note we also still have a few free() left around in the code base to cleanup too. > codebase, and I do properly check the return code of all calls to *alloc. > If you insist on those calls being replaced, I can do that, but I didn't > before submitting because I knew the current calls were correct and > didn't want to introduce bugs into the code. yeah, I think it's better to fix them before. In any case we have a week of bug fix only before the next release in a week, so best is to fix them now, wait for the actual reviews and then resubmit patches for inclusion. thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0