[libvirt] [PATCH] test: fix commantest under autobuild.sh
by Eric Blake
* tests/commandtest.c (mymain): Kill off any leaked-in fds.
* autobuild.sh: Don't leak fds.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Autobuild failed because it triggered a commandtest failure where
commandtest was too sensitive to the environment. Fix the problem
at both ends - autobuild shouldn't leak, and commandtest should
work even when somebody else leaked.
autobuild.sh | 2 +-
tests/commandtest.c | 6 ++++++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/autobuild.sh b/autobuild.sh
index 8248a59..91e2ab2 100755
--- a/autobuild.sh
+++ b/autobuild.sh
@@ -38,7 +38,7 @@ make install
exec 3>&1
st=$(
exec 4>&1 >&3
- { make check syntax-check 2>&1; echo $? >&4; } | tee "$RESULTS"
+ { make check syntax-check 2>&1 3>&- 4>&-; echo $? >&4; } | tee "$RESULTS"
)
exec 3>&-
test "$st" = 0
diff --git a/tests/commandtest.c b/tests/commandtest.c
index a1bcf68..333dd4d 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -688,6 +688,12 @@ mymain(int argc, char **argv)
if (chdir("/tmp") < 0)
return(EXIT_FAILURE);
+ /* Kill off any inherited fds that might interfere with our
+ * testing. */
+ close(3);
+ close(4);
+ close(5);
+
virInitialize();
const char *const newenv[] = {
--
1.7.3.2
14 years, 5 months
[libvirt] [v2] qemu: Introduce two new job types
by Osier Yang
Currently, all of domain "save/dump/managed save/migration"
use the same function "qemudDomainWaitForMigrationComplete"
to wait the job finished, but the error messages are all
about "migration", e.g. when a domain saving job is canceled
by user, "migration was cancled by client" will be throwed as
an error message, which will be confused for user.
As a solution, intoduce two new job types(QEMU_JOB_SAVE,
QEMU_JOB_DUMP), and set "priv->jobActive" to "QEMU_JOB_SAVE"
before saving, to "QEMU_JOB_DUMP" before dumping, so that we
could get the real job type in
"qemudDomainWaitForMigrationComplete", and give more clear
message further.
And as It's not important to figure out what's the exact job
is in the DEBUG and WARN log, also we don't need translated
string in logs, simply repace "migration" with "job" in some
statements.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++---------
1 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 19ce9a6..d5af0df 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -104,6 +104,8 @@ enum qemuDomainJob {
QEMU_JOB_UNSPECIFIED,
QEMU_JOB_MIGRATION_OUT,
QEMU_JOB_MIGRATION_IN,
+ QEMU_JOB_SAVE,
+ QEMU_JOB_DUMP,
};
enum qemuDomainJobSignals {
@@ -5389,21 +5391,37 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
struct timeval now;
int rc;
+ const char *job;
+
+ switch (priv->jobActive) {
+ case QEMU_JOB_MIGRATION_OUT:
+ job = _("migration job");
+ break;
+ case QEMU_JOB_SAVE:
+ job = _("domain save job");
+ break;
+ case QEMU_JOB_DUMP:
+ job = _("domain core dump job");
+ break;
+ default:
+ job = _("job");
+ }
+
if (!virDomainObjIsActive(vm)) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit during migration"));
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s: %s",
+ job, _("guest unexpectedly quit"));
goto cleanup;
}
if (priv->jobSignals & QEMU_JOB_SIGNAL_CANCEL) {
priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL;
- VIR_DEBUG0("Cancelling migration at client request");
+ VIR_DEBUG0("Cancelling job at client request");
qemuDomainObjEnterMonitorWithDriver(driver, vm);
rc = qemuMonitorMigrateCancel(priv->mon);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (rc < 0) {
- VIR_WARN0("Unable to cancel migration");
+ VIR_WARN0("Unable to cancel job");
}
} else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) {
priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND;
@@ -5427,8 +5445,8 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
* guest to die
*/
if (!virDomainObjIsActive(vm)) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit during migration"));
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s: %s",
+ job, _("guest unexpectedly quit"));
goto cleanup;
}
@@ -5459,7 +5477,7 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE:
priv->jobInfo.type = VIR_DOMAIN_JOB_NONE;
qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration is not active"));
+ "%s: %s", job, _("is not active"));
break;
case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE:
@@ -5480,13 +5498,13 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
case QEMU_MONITOR_MIGRATION_STATUS_ERROR:
priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED;
qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration unexpectedly failed"));
+ "%s: %s", job, _("unexpectedly failed"));
break;
case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED:
priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED;
qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration was cancelled by client"));
+ "%s: %s", job, _("canceled by client"));
break;
}
@@ -5606,6 +5624,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
goto endjob;
}
+ priv->jobActive = QEMU_JOB_SAVE;
+
memset(&priv->jobInfo, 0, sizeof(priv->jobInfo));
priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED;
@@ -6198,6 +6218,8 @@ static int qemudDomainCoreDump(virDomainPtr dom,
goto endjob;
}
+ priv->jobActive = QEMU_JOB_DUMP;
+
/* Migrate will always stop the VM, so the resume condition is
independent of whether the stop command is issued. */
resume = (vm->state == VIR_DOMAIN_RUNNING);
--
1.7.3.2
14 years, 5 months
[libvirt] [PATCH] daemon: plug a memory leak
by Eric Blake
* daemon/libvirtd.c (qemudFreeClient): Avoid a leak.
(qemudDispatchServer): Avoid null dereference.
---
I keep finding more of these.
daemon/libvirtd.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 791b3dc..2914f2f 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1410,7 +1410,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket
/* Client is running as root, so disable auth */
if (uid == 0) {
VIR_INFO(_("Turn off polkit auth for privileged client pid %d from %s"),
- pid, addrstr);
+ pid, client->addrstr);
client->auth = REMOTE_AUTH_NONE;
}
}
@@ -1451,7 +1451,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket
} else {
PROBE(CLIENT_TLS_FAIL, "fd=%d", client->fd);
VIR_ERROR(_("TLS handshake failed for client %s: %s"),
- addrstr, gnutls_strerror (ret));
+ client->addrstr, gnutls_strerror (ret));
goto error;
}
}
@@ -2283,6 +2283,7 @@ static void qemudFreeClient(struct qemud_client *client) {
if (client->conn)
virConnectClose(client->conn);
virMutexDestroy(&client->lock);
+ VIR_FREE(client->addrstr);
VIR_FREE(client);
}
--
1.7.3.2
14 years, 5 months
[libvirt] [PATCH] qemu: Introduce two new job types
by Osier Yang
Currently, all of domain "save/dump/managed save/migration"
use the same function "qemudDomainWaitForMigrationComplete"
to wait the job finished, but the error messages are all
about "migration", e.g. when a domain saving job is canceled
by user, "migration was cancled by client" will be throwed as
an error message, which will be confused for user.
As a solution, intoduce two new job types(QEMU_JOB_SAVE,
QEMU_JOB_DUMP), and set "priv->jobActive" to "QEMU_JOB_SAVE"
before saving, to "QEMU_JOB_DUMP" before dumping, so that we
could get the real job type in
"qemudDomainWaitForMigrationComplete", and give more clear
message further.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 39 ++++++++++++++++++++++++++++++---------
1 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 19ce9a6..aed48f4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -104,6 +104,8 @@ enum qemuDomainJob {
QEMU_JOB_UNSPECIFIED,
QEMU_JOB_MIGRATION_OUT,
QEMU_JOB_MIGRATION_IN,
+ QEMU_JOB_SAVE,
+ QEMU_JOB_DUMP,
};
enum qemuDomainJobSignals {
@@ -5389,21 +5391,36 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
struct timeval now;
int rc;
+ const char *job;
+
+ switch (priv->jobActive) {
+ case QEMU_JOB_MIGRATION_OUT:
+ job = "migration";
+ break;
+ case QEMU_JOB_SAVE:
+ job = "domain saving";
+ break;
+ case QEMU_JOB_DUMP:
+ job = "domain core dump";
+ break;
+ default:
+ job = "job";
+ }
if (!virDomainObjIsActive(vm)) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit during migration"));
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("guest unexpectedly quit during %s"), job);
goto cleanup;
}
if (priv->jobSignals & QEMU_JOB_SIGNAL_CANCEL) {
priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL;
- VIR_DEBUG0("Cancelling migration at client request");
+ VIR_DEBUG("Cancelling %s at client request", job);
qemuDomainObjEnterMonitorWithDriver(driver, vm);
rc = qemuMonitorMigrateCancel(priv->mon);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (rc < 0) {
- VIR_WARN0("Unable to cancel migration");
+ VIR_WARN("Unable to cancel %s", job);
}
} else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) {
priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND;
@@ -5427,8 +5444,8 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
* guest to die
*/
if (!virDomainObjIsActive(vm)) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit during migration"));
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("guest unexpectedly quit during %s"), job);
goto cleanup;
}
@@ -5459,7 +5476,7 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE:
priv->jobInfo.type = VIR_DOMAIN_JOB_NONE;
qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration is not active"));
+ _("%s is not active"), job);
break;
case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE:
@@ -5480,13 +5497,13 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr
case QEMU_MONITOR_MIGRATION_STATUS_ERROR:
priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED;
qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration unexpectedly failed"));
+ _("%s unexpectedly failed"), job);
break;
case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED:
priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED;
qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration was cancelled by client"));
+ _("%s was cancelled by client"), job);
break;
}
@@ -5606,6 +5623,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
goto endjob;
}
+ priv->jobActive = QEMU_JOB_SAVE;
+
memset(&priv->jobInfo, 0, sizeof(priv->jobInfo));
priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED;
@@ -6198,6 +6217,8 @@ static int qemudDomainCoreDump(virDomainPtr dom,
goto endjob;
}
+ priv->jobActive = QEMU_JOB_DUMP;
+
/* Migrate will always stop the VM, so the resume condition is
independent of whether the stop command is issued. */
resume = (vm->state == VIR_DOMAIN_RUNNING);
--
1.7.3.2
14 years, 5 months
[libvirt] ANNOUNCE: ruby-libvirt bindings 0.3.0
by Chris Lalancette
All,
I'm pleased to announce the release of 0.3.0 of the ruby-libvirt bindings.
This release has a number of updates:
* Implementation of Libvirt::open_auth, Libvirt::event_register_impl
* Updated Connect class, implementing conn.compare_cpu, conn.baseline_cpu,
conn.domain_event_register_any, conn.domain_event_deregister_any,
conn.domain_event_register, conn.domain_event_deregister, and
conn.create_domain_xml.
* Updated Domain class, implementing dom.get_vcpus, dom.update_device,
dom.scheduler_type, dom.scheduler_parameters, dom.scheduler_parameters=,
dom.num_vcpus, dom.vcpus_flags=, and dom.qemu_monitor_command.
* Updated Interface class, implementing interface.free
* Many potential memory leaks have been fixed.
* Many bugfixes.
* Documentation update of many methods, including all of the lookup methods
that were missing before.
With the above list, almost all of the APIs up to and including libvirt 0.8.6
are covered. There are a few that I have not yet had time to implement; you
can look at the list in the README file in the source tree to see which ones
are missing.
The tarballs and source RPMs are here: http://libvirt.org/ruby/download/
The git repository is here: http://libvirt.org/git/?p=ruby-libvirt.git;a=summary
The documentation for all of the APIs are here: http://libvirt.org/ruby/api/index.html
I've uploaded some examples on how to use the bindings:
http://libvirt.org/ruby/documentation.html
I've also pushed a new gem to rubygems.org; you should be able to use
'gem install ruby-libvirt' (if you haven't installed them before) or
'gem update ruby-libvirt' (if you have them installed already).
If you have time, and are interested in the bindings, please give these new
ones a whirl and let me know if they work for you. Note that release 0.3.0
is intended to be fully backwards compatible with release 0.2.0 and 0.1.0.
If you find that they are not, or find any other problems with the ruby
bindings, please report it to the libvirt development mailing list
(and CC me) so that I can take a look at them.
Thanks,
--
Chris Lalancette
14 years, 5 months
[libvirt] [PATCH] Convert dhcpStartDhcpDaemon from virRun to virCommand
by Laine Stump
This is pretty straightforward - even though dnsmasq gets daemonized
and uses a pid file, those things are both handled by the dnsmasq
binary itself. And libvirt doesn't need any of the output of the
dnsmasq command either, so we just setup the args and call
virRun(). Mainly it was just a (mostly) mechanical job of replacing
the APPEND_ARG() macro (and some other *printfs()) with
virCommandAddArg*().
---
src/network/bridge_driver.c | 238 +++++++++++++++----------------------------
1 files changed, 80 insertions(+), 158 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 62639e4..9802222 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -51,6 +51,7 @@
#include "event.h"
#include "buf.h"
#include "util.h"
+#include "command.h"
#include "memory.h"
#include "uuid.h"
#include "iptables.h"
@@ -390,25 +391,15 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
static int
networkBuildDnsmasqArgv(virNetworkObjPtr network,
const char *pidfile,
- const char ***argv) {
- int i, len, r;
+ virCommandPtr cmd) {
+ int r, ret = -1;
int nbleases = 0;
- char *pidfileArg;
- char buf[1024];
- unsigned int ranges;
- char *ipAddr;
-
- /*
- * For static-only DHCP, i.e. with no range but at least one host element,
- * we have to add a special --dhcp-range option to enable the service in
- * dnsmasq.
- */
- ranges = network->def->nranges;
- if (!ranges && network->def->nhosts)
- ranges = 1;
+ char *bridgeaddr;
+ if (!(bridgeaddr = virSocketFormatAddr(&network->def->ipAddress)))
+ goto cleanup;
/*
- * NB, be careful about syntax for dnsmasq options in long format
+ * NB, be careful about syntax for dnsmasq options in long format.
*
* If the flag has a mandatory argument, it can be given using
* either syntax:
@@ -422,66 +413,27 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* --foo=bar
*
* It is hard to determine whether a flag is optional or not,
- * without reading the dnsmasq source :-( The manpages is not
- * very explicit on this
+ * without reading the dnsmasq source :-( The manpage is not
+ * very explicit on this.
*/
- len =
- 1 + /* dnsmasq */
- 1 + /* --strict-order */
- 1 + /* --bind-interfaces */
- (network->def->domain?2:0) + /* --domain name */
- 2 + /* --pid-file /var/run/libvirt/network/$NAME.pid */
- 2 + /* --conf-file "" */
- /*2 + *//* --interface virbr0 */
- 2 + /* --except-interface lo */
- 2 + /* --listen-address 10.0.0.1 */
- (2 * ranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
- /* --dhcp-lease-max=xxx if needed */
- (network->def->nranges ? 1 : 0) +
- /* --dhcp-no-override if needed */
- (ranges ? 1 : 0) +
- /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */
- (network->def->nhosts > 0 ? 1 : 0) +
- /* --enable-tftp --tftp-root /srv/tftp */
- (network->def->tftproot ? 3 : 0) +
- /* --dhcp-boot pxeboot.img[,,12.34.56.78] */
- (network->def->bootfile ? 2 : 0) +
- 1; /* NULL */
-
- if (VIR_ALLOC_N(*argv, len) < 0)
- goto no_memory;
-
-#define APPEND_ARG(v, n, s) do { \
- if (!((v)[(n)] = strdup(s))) \
- goto no_memory; \
- } while (0)
-
-#define APPEND_ARG_LIT(v, n, s) \
- (v)[(n)] = s
-
- i = 0;
-
- APPEND_ARG(*argv, i++, DNSMASQ);
-
/*
* Needed to ensure dnsmasq uses same algorithm for processing
* multiple namedriver entries in /etc/resolv.conf as GLibC.
*/
- APPEND_ARG(*argv, i++, "--strict-order");
- APPEND_ARG(*argv, i++, "--bind-interfaces");
+ virCommandAddArg(cmd, "--strict-order");
+ virCommandAddArg(cmd, "--bind-interfaces");
if (network->def->domain) {
- APPEND_ARG(*argv, i++, "--domain");
- APPEND_ARG(*argv, i++, network->def->domain);
+ virCommandAddArg(cmd, "--domain");
+ virCommandAddArg(cmd, network->def->domain);
}
- if (virAsprintf(&pidfileArg, "--pid-file=%s", pidfile) < 0)
- goto no_memory;
- APPEND_ARG_LIT(*argv, i++, pidfileArg);
+ virCommandAddArgPair(cmd, "--pid-file", pidfile);
- APPEND_ARG(*argv, i++, "--conf-file=");
- APPEND_ARG(*argv, i++, "");
+ /* *no* conf file */
+ virCommandAddArg(cmd, "--conf-file=");
+ virCommandAddArg(cmd, "");
/*
* XXX does not actually work, due to some kind of
@@ -489,166 +441,140 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* interface. A sleep(10) makes it work, but that's
* clearly not practical
*
- * APPEND_ARG(*argv, i++, "--interface");
- * APPEND_ARG(*argv, i++, network->def->bridge);
+ * virCommandAddArg(cmd, "--interface");
+ * virCommandAddArg(cmd, network->def->bridge);
*/
- APPEND_ARG(*argv, i++, "--listen-address");
- if (!(ipAddr = virSocketFormatAddr(&network->def->ipAddress)))
- goto error;
- APPEND_ARG_LIT(*argv, i++, ipAddr);
+ virCommandAddArg(cmd, "--listen-address");
+ virCommandAddArg(cmd, bridgeaddr);
- APPEND_ARG(*argv, i++, "--except-interface");
- APPEND_ARG(*argv, i++, "lo");
+ virCommandAddArg(cmd, "--except-interface");
+ virCommandAddArg(cmd, "lo");
for (r = 0 ; r < network->def->nranges ; r++) {
char *saddr = virSocketFormatAddr(&network->def->ranges[r].start);
if (!saddr)
- goto error;
+ goto cleanup;
char *eaddr = virSocketFormatAddr(&network->def->ranges[r].end);
if (!eaddr) {
VIR_FREE(saddr);
- goto error;
+ goto cleanup;
}
- char *range;
- int rc = virAsprintf(&range, "%s,%s", saddr, eaddr);
+ virCommandAddArg(cmd, "--dhcp-range");
+ virCommandAddArgFormat(cmd, "%s,%s", saddr, eaddr);
VIR_FREE(saddr);
VIR_FREE(eaddr);
- if (rc < 0)
- goto no_memory;
- APPEND_ARG(*argv, i++, "--dhcp-range");
- APPEND_ARG_LIT(*argv, i++, range);
nbleases += virSocketGetRange(&network->def->ranges[r].start,
&network->def->ranges[r].end);
}
+ /*
+ * For static-only DHCP, i.e. with no range but at least one host element,
+ * we have to add a special --dhcp-range option to enable the service in
+ * dnsmasq.
+ */
if (!network->def->nranges && network->def->nhosts) {
- char *ipaddr = virSocketFormatAddr(&network->def->ipAddress);
- if (!ipaddr)
- goto error;
- char *range;
- int rc = virAsprintf(&range, "%s,static", ipaddr);
- VIR_FREE(ipaddr);
- if (rc < 0)
- goto no_memory;
-
- APPEND_ARG(*argv, i++, "--dhcp-range");
- APPEND_ARG_LIT(*argv, i++, range);
+ virCommandAddArg(cmd, "--dhcp-range");
+ virCommandAddArgFormat(cmd, "%s,static", bridgeaddr);
}
if (network->def->nranges > 0) {
- snprintf(buf, sizeof(buf), "--dhcp-lease-max=%d", nbleases);
- APPEND_ARG(*argv, i++, buf);
+ virCommandAddArgFormat(cmd, "--dhcp-lease-max=%d", nbleases);
}
- if (ranges)
- APPEND_ARG(*argv, i++, "--dhcp-no-override");
+ if (network->def->nranges || network->def->nhosts)
+ virCommandAddArg(cmd, "--dhcp-no-override");
if (network->def->nhosts > 0) {
- dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR);
- char *hostsfileArg;
-
- if (dctx == NULL)
- goto no_memory;
+ dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
+ DNSMASQ_STATE_DIR);
+ if (dctx == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ }
if (networkSaveDnsmasqHostsfile(network, dctx, false)) {
- if (virAsprintf(&hostsfileArg, "--dhcp-hostsfile=%s", dctx->hostsfile->path) < 0) {
- dnsmasqContextFree(dctx);
- goto no_memory;
- }
- APPEND_ARG_LIT(*argv, i++, hostsfileArg);
+ virCommandAddArgPair(cmd, "--dhcp-hostsfile",
+ dctx->hostsfile->path);
}
-
dnsmasqContextFree(dctx);
}
if (network->def->tftproot) {
- APPEND_ARG(*argv, i++, "--enable-tftp");
- APPEND_ARG(*argv, i++, "--tftp-root");
- APPEND_ARG(*argv, i++, network->def->tftproot);
+ virCommandAddArg(cmd, "--enable-tftp");
+ virCommandAddArg(cmd, "--tftp-root");
+ virCommandAddArg(cmd, network->def->tftproot);
}
if (network->def->bootfile) {
- char *ipaddr = NULL;
+
+ virCommandAddArg(cmd, "--dhcp-boot");
if (VIR_SOCKET_HAS_ADDR(&network->def->bootserver)) {
- if (!(ipaddr = virSocketFormatAddr(&network->def->bootserver)))
- goto error;
- }
- char *boot;
- int rc = virAsprintf(&boot, "%s%s%s",
- network->def->bootfile,
- ipaddr ? ",," : "",
- ipaddr ? ipaddr : "");
- VIR_FREE(ipaddr);
- if (rc < 0)
- goto no_memory;
+ char *bootserver = virSocketFormatAddr(&network->def->bootserver);
- APPEND_ARG(*argv, i++, "--dhcp-boot");
- APPEND_ARG_LIT(*argv, i++, boot);
+ if (!bootserver)
+ goto cleanup;
+ virCommandAddArgFormat(cmd, "%s%s%s",
+ network->def->bootfile, ",,", bootserver);
+ VIR_FREE(bootserver);
+ } else {
+ virCommandAddArg(cmd, network->def->bootfile);
+ }
}
-#undef APPEND_ARG
-
- return 0;
-
- no_memory:
- virReportOOMError();
- error:
- if (*argv) {
- for (i = 0; (*argv)[i]; i++)
- VIR_FREE((*argv)[i]);
- VIR_FREE(*argv);
- }
- return -1;
+ ret = 0;
+cleanup:
+ VIR_FREE(bridgeaddr);
+ return ret;
}
static int
dhcpStartDhcpDaemon(virNetworkObjPtr network)
{
- const char **argv;
- char *pidfile;
- int ret = -1, i, err;
+ virCommandPtr cmd = NULL;
+ char *pidfile = NULL;
+ int ret = -1, err;
network->dnsmasqPid = -1;
if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot start dhcp daemon without IPv4 address for server"));
- return -1;
+ goto cleanup;
}
if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) {
virReportSystemError(err,
_("cannot create directory %s"),
NETWORK_PID_DIR);
- return -1;
+ goto cleanup;
}
if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) {
virReportSystemError(err,
_("cannot create directory %s"),
NETWORK_STATE_DIR);
- return -1;
+ goto cleanup;
}
if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) {
virReportOOMError();
- return -1;
+ goto cleanup;
}
- argv = NULL;
- if (networkBuildDnsmasqArgv(network, pidfile, &argv) < 0) {
+ cmd = virCommandNew(DNSMASQ);
+ if (networkBuildDnsmasqArgv(network, pidfile, cmd) < 0) {
VIR_FREE(pidfile);
- return -1;
+ goto cleanup;
}
- if (virRun(argv, NULL) < 0)
+ if (virCommandRun(cmd, NULL) < 0)
goto cleanup;
/*
- * There really is no race here - when dnsmasq daemonizes,
- * its leader process stays around until its child has
- * actually written its pidfile. So by time virRun exits
- * it has waitpid'd and guaranteed the proess has started
- * and written a pid
+ * There really is no race here - when dnsmasq daemonizes, its
+ * leader process stays around until its child has actually
+ * written its pidfile. So by time virCommandRun exits it has
+ * waitpid'd and guaranteed the proess has started and written a
+ * pid
*/
if (virFileReadPid(NETWORK_PID_DIR, network->def->name,
@@ -656,13 +582,9 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network)
goto cleanup;
ret = 0;
-
cleanup:
VIR_FREE(pidfile);
- for (i = 0; argv[i]; i++)
- VIR_FREE(argv[i]);
- VIR_FREE(argv);
-
+ virCommandFree(cmd);
return ret;
}
--
1.7.3.2
14 years, 5 months
[libvirt] [PATCH 0/4] more memory leak patches
by Eric Blake
More in the ongoing battle to make libvirtd not so leaky.
Also related, and still waiting on a review for:
virExec (and thus virCommand) have undefined behavior
https://www.redhat.com/archives/libvir-list/2010-December/msg00242.html
avoid matchpathcon memory leak (although libselinux 2.0.97 finally
patched that leak upstream)
https://www.redhat.com/archives/libvir-list/2010-December/msg00001.html
The leak in qemu_conf covered in patch 3 was introduced with the
conversion to virCommand; if you like patch 4, I'd rather combine
those into one patch (but patch 3 is a minimal plug).
Eric Blake (4):
tests: plug memory leaks
conf: plug memory leaks
qemu: plug memory leak
command: ease use with virBuffer
src/conf/domain_conf.c | 10 ++++++++++
src/libvirt_private.syms | 2 ++
src/qemu/qemu_conf.c | 42 +++++++++---------------------------------
src/util/command.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/util/command.h | 17 +++++++++++++++++
tests/qemuxml2argvtest.c | 3 +--
6 files changed, 82 insertions(+), 35 deletions(-)
--
1.7.3.2
14 years, 5 months
[libvirt] [PATCH] virExec: avoid undefined behavior
by Eric Blake
* src/util/util.c (__virExec): Don't use FD_ISSET on out-of-bounds fd.
---
Noticed this one by inspection, while investigating
https://bugzilla.redhat.com/show_bug.cgi?id=659855
Don't know if it's the root cause of the crash in that bug, though.
src/util/util.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 79ca5d3..1b5bc68 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -570,8 +570,7 @@ __virExec(const char *const*argv,
i != null &&
i != childout &&
i != childerr &&
- (!keepfd ||
- !FD_ISSET(i, keepfd))) {
+ (!keepfd || (i < FD_SETSIZE && !FD_ISSET(i, keepfd)))) {
tmpfd = i;
VIR_FORCE_CLOSE(tmpfd);
}
--
1.7.3.2
14 years, 5 months
[libvirt] [PATCH] esx: Refactor storage pool type lookup into a function
by Matthias Bolte
---
src/esx/esx_storage_driver.c | 130 ++++++++++++++++++------------------------
1 files changed, 56 insertions(+), 74 deletions(-)
diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c
index 83b53fb..9165f7a 100644
--- a/src/esx/esx_storage_driver.c
+++ b/src/esx/esx_storage_driver.c
@@ -49,6 +49,58 @@ verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN);
+static int
+esxStoragePoolLookupType(esxVI_Context *ctx, const char *poolName,
+ int *poolType)
+{
+ int result = -1;
+ esxVI_String *propertyNameList = NULL;
+ esxVI_ObjectContent *datastore = NULL;
+ esxVI_DynamicProperty *dynamicProperty = NULL;
+ esxVI_DatastoreInfo *datastoreInfo = NULL;
+
+ if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 ||
+ esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, &datastore,
+ esxVI_Occurrence_RequiredItem) < 0) {
+ goto cleanup;
+ }
+
+ for (dynamicProperty = datastore->propSet; dynamicProperty != NULL;
+ dynamicProperty = dynamicProperty->_next) {
+ if (STREQ(dynamicProperty->name, "info")) {
+ if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val,
+ &datastoreInfo) < 0) {
+ goto cleanup;
+ }
+
+ break;
+ }
+ }
+
+ if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
+ *poolType = VIR_STORAGE_POOL_DIR;
+ } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
+ *poolType = VIR_STORAGE_POOL_NETFS;
+ } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
+ *poolType = VIR_STORAGE_POOL_FS;
+ } else {
+ ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("DatastoreInfo has unexpected type"));
+ goto cleanup;
+ }
+
+ result = 0;
+
+ cleanup:
+ esxVI_String_Free(&propertyNameList);
+ esxVI_ObjectContent_Free(&datastore);
+ esxVI_DatastoreInfo_Free(&datastoreInfo);
+
+ return result;
+}
+
+
+
static virDrvOpenStatus
esxStorageOpen(virConnectPtr conn,
virConnectAuthPtr auth ATTRIBUTE_UNUSED,
@@ -908,10 +960,6 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc,
{
virStorageVolPtr volume = NULL;
esxPrivate *priv = pool->conn->storagePrivateData;
- esxVI_String *propertyNameList = NULL;
- esxVI_ObjectContent *datastore = NULL;
- esxVI_DynamicProperty *dynamicProperty = NULL;
- esxVI_DatastoreInfo *datastoreInfo = NULL;
virStoragePoolDef poolDef;
virStorageVolDefPtr def = NULL;
char *tmp;
@@ -938,36 +986,8 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc,
return NULL;
}
- /* Lookup storage pool type */
- if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 ||
- esxVI_LookupDatastoreByName(priv->primary, pool->name,
- propertyNameList, &datastore,
- esxVI_Occurrence_RequiredItem) < 0) {
- goto cleanup;
- }
-
- for (dynamicProperty = datastore->propSet; dynamicProperty != NULL;
- dynamicProperty = dynamicProperty->_next) {
- if (STREQ(dynamicProperty->name, "info")) {
- if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val,
- &datastoreInfo) < 0) {
- goto cleanup;
- }
-
- break;
- }
- }
-
- if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
- poolDef.type = VIR_STORAGE_POOL_DIR;
- } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
- poolDef.type = VIR_STORAGE_POOL_NETFS;
- } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
- poolDef.type = VIR_STORAGE_POOL_FS;
- } else {
- ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
- _("DatastoreInfo has unexpected type"));
- goto cleanup;
+ if (esxStoragePoolLookupType(priv->primary, pool->name, &poolDef.type) < 0) {
+ return NULL;
}
/* Parse config */
@@ -1140,9 +1160,6 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc,
virtualDiskSpec->adapterType = NULL;
}
- esxVI_String_Free(&propertyNameList);
- esxVI_ObjectContent_Free(&datastore);
- esxVI_DatastoreInfo_Free(&datastoreInfo);
virStorageVolDefFree(def);
VIR_FREE(unescapedDatastorePath);
VIR_FREE(unescapedDirectoryName);
@@ -1216,10 +1233,6 @@ static char *
esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags)
{
esxPrivate *priv = volume->conn->storagePrivateData;
- esxVI_String *propertyNameList = NULL;
- esxVI_ObjectContent *datastore = NULL;
- esxVI_DynamicProperty *dynamicProperty = NULL;
- esxVI_DatastoreInfo *datastoreInfo = NULL;
virStoragePoolDef pool;
char *datastorePath = NULL;
esxVI_FileInfo *fileInfo = NULL;
@@ -1238,36 +1251,8 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags)
return NULL;
}
- /* Lookup storage pool type */
- if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 ||
- esxVI_LookupDatastoreByName(priv->primary, volume->pool,
- propertyNameList, &datastore,
- esxVI_Occurrence_RequiredItem) < 0) {
- goto cleanup;
- }
-
- for (dynamicProperty = datastore->propSet; dynamicProperty != NULL;
- dynamicProperty = dynamicProperty->_next) {
- if (STREQ(dynamicProperty->name, "info")) {
- if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val,
- &datastoreInfo) < 0) {
- goto cleanup;
- }
-
- break;
- }
- }
-
- if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
- pool.type = VIR_STORAGE_POOL_DIR;
- } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
- pool.type = VIR_STORAGE_POOL_NETFS;
- } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
- pool.type = VIR_STORAGE_POOL_FS;
- } else {
- ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
- _("DatastoreInfo has unexpected type"));
- goto cleanup;
+ if (esxStoragePoolLookupType(priv->primary, volume->pool, &pool.type) < 0) {
+ return NULL;
}
/* Lookup file info */
@@ -1320,9 +1305,6 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags)
xml = virStorageVolDefFormat(&pool, &def);
cleanup:
- esxVI_String_Free(&propertyNameList);
- esxVI_ObjectContent_Free(&datastore);
- esxVI_DatastoreInfo_Free(&datastoreInfo);
VIR_FREE(datastorePath);
esxVI_FileInfo_Free(&fileInfo);
VIR_FREE(def.key);
--
1.7.0.4
14 years, 5 months
