[libvirt] virDomainInterfaceStats why is there a size?
by Stefan de Konink
Another simple question, what is the reasoning about the size field in
this call. I would really be a happy boy if anyone said:
if you put in path == NULL, it will fill your stats structure up to size
interfaces.
...but since this is not the case (yet) what is the reason behind it?
Stefan
15 years, 10 months
[libvirt] PATCH: 0/28: Thread safety for libvirtd daemon and drivers
by Daniel P. Berrange
The following huge series of patches adds thread safety for the libvirtd
daemon and drivers, and makes the daemon multi-threaded in processing
RPC calls. This enables multiple clients to be processed in parallel,
without blocking each other. It does not change the thread rules for the
virConnectPtr object though, so each individual client is still serialized.
There are two core places where we have to have synchronization in the
threading model this patch series introduces
- The libvirt daemon code
- A single global server lock (aka struct qemud_server)
- One lock per client connection (aka struct qemud_client)
- The driver implementations
- One lock per driver (aka QEMU, LXC, Test, UML, OpenVZ, Network,
Storage, Node Devices)
- One lock per primary object in a driver (virDomainObjPtr,
virNetworkObjPtr, virStoragePoolObjPtr, virNodeDeviceObjptr
instances)
For most cases, the big global server / driver locks are only held while
obtaining one of the finer grained locks. This gives a fairly good level
of concurrency for operations touching different objects. Once this core
infrastructure is merged, it will be possible to iterate on impl of
drivers to reduce the time locks are held - eg avoid holding a lock while
talking to the QEMU monitor interface.
To try and make it easier to spot thread locking problems this series
refactors alot of methods so that there is only a single return point
where the unlock call can be placed, rather than multiple return point
which increases the chances of missing an unlock call.
This touches a huge amount of code, so I'd like to get this all merged
ASAP as it'll be really hard to keep it synced with ongoing changes.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 10 months
[libvirt] Re: [PATCH/RFC] kvm/qemu vs libvirtd restarts
by Daniel P. Berrange
Moving thread to libvir-list....
On Tue, Oct 07, 2008 at 06:12:33PM +0200, Guido G?nther wrote:
> On Fri, Sep 26, 2008 at 04:47:05PM +0100, Daniel P. Berrange wrote:
> > On Fri, Sep 26, 2008 at 05:41:02PM +0200, Guido G?nther wrote:
> >
> > We've got this problem sorted in the 'lxc' driver and need to apply
> > the same approach to the QEMU driver. The problem was always finding
> > the Pseduo-TTY/PID for the monitor after restart, and associating the
> > live config with it. We've "solved" that in LXC driver by savingt the
> > live XML & PID to /var/run/libvirt/lxc/. It basically needs the same
> > approach to be applied to the QEMU daemons. In addition the DNSmasq
> > deamon needs adapting for simiarl reasons.
>
> Attached is a very early patch. It keeps the qemu instances running and
> writes out their states upon shutdown. Reconnecting (and therefore sending
> monitor commands, etc.) works but since stdout/stderr don't get picked
> up again yet we don't handle virEventAddhandle. I'll grab them from
> /proc/<pid>/fd/{1,2} later. Network state is missing too, but since this
> is moving into a separate file it's becoming a different matter anyway.
First off, thanks for working on this! A few general comments, and then
I'll put rest inline..
For stdout/err, it need to be setup so that the logfile is dup()'d onto
the QEMU process' file descriptors directly, and get libvirt outof the
business of I/O forwarding. This avoids any complications with re-attaching
to stdout/err, and loosing any log data while libvirt isn't running.
This also needs to be implemented with the assumption that the daemon
can & will crash any time. So putting the state-saving hooks into the
qemudShutdown() method won't be sufficient. We need to write out the
state we need when initially starting the VM, and then update it any
time we hot-plug/unplug devices.
Ignoring network state is fine - we can address that separately..
> Subject: [PATCH] let qemu/kvm survive libvirtd restarts
>
> ---
> src/domain_conf.c | 1 +
> src/domain_conf.h | 1 +
> src/qemu_conf.h | 1 +
> src/qemu_driver.c | 264 +++++++++++++++++++++++++++++++++++++++++++++++++----
> src/util.c | 4 +-
> src/util.h | 1 +
> 6 files changed, 254 insertions(+), 18 deletions(-)
>
> diff --git a/src/domain_conf.c b/src/domain_conf.c
> index 6a35064..aa102f6 100644
> --- a/src/domain_conf.c
> +++ b/src/domain_conf.c
> @@ -420,6 +420,7 @@ void virDomainObjFree(virDomainObjPtr dom)
> virDomainDefFree(dom->def);
> virDomainDefFree(dom->newDef);
>
> + VIR_FREE(dom->monitorpath);
> VIR_FREE(dom->vcpupids);
>
> VIR_FREE(dom);
> diff --git a/src/domain_conf.h b/src/domain_conf.h
> index 632e5ad..1ac1561 100644
> --- a/src/domain_conf.h
> +++ b/src/domain_conf.h
> @@ -448,6 +448,7 @@ struct _virDomainObj {
> int stdout_fd;
> int stderr_fd;
> int monitor;
> + char *monitorpath;
I think we can avoid needing this. If we write out the staste the
moment the VM is started, we don't need to carry this around in
memory. Alternatively, since we're writing all stdout/err data to
the logfile, we could simply re-parse the log to extract the
monitor path upon restart.
> int logfile;
> int pid;
> int state;
> diff --git a/src/qemu_conf.h b/src/qemu_conf.h
> index 88dfade..f47582e 100644
> --- a/src/qemu_conf.h
> +++ b/src/qemu_conf.h
> @@ -62,6 +62,7 @@ struct qemud_driver {
> char *networkConfigDir;
> char *networkAutostartDir;
> char *logDir;
> + char *stateDir;
> unsigned int vncTLS : 1;
> unsigned int vncTLSx509verify : 1;
> char *vncTLSx509certdir;
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 806608d..87789a9 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -172,6 +172,181 @@ void qemudAutostartConfigs(struct qemud_driver *driver) {
> }
>
> #ifdef WITH_LIBVIRTD
> +
> +static int
> +qemudFileWriteMonitor(const char *dir,
> + const char *name,
> + const char *path)
> +{
> + int rc;
> + int fd;
> + FILE *file = NULL;
> + char *monitorfile = NULL;
> +
> + if ((rc = virFileMakePath(dir)))
> + goto cleanup;
> +
> + if (asprintf(&monitorfile, "%s/%s.monitor", dir, name) < 0) {
> + rc = ENOMEM;
> + goto cleanup;
> + }
> +
> + if ((fd = open(monitorfile,
> + O_WRONLY | O_CREAT | O_TRUNC,
> + S_IRUSR | S_IWUSR)) < 0) {
> + rc = errno;
> + goto cleanup;
> + }
> +
> + if (!(file = fdopen(fd, "w"))) {
> + rc = errno;
> + close(fd);
> + goto cleanup;
> + }
> +
> + if (fprintf(file, "%s", path) < 0) {
> + rc = errno;
> + goto cleanup;
> + }
> +
> + rc = 0;
> +
> +cleanup:
> + if (file &&
> + fclose(file) < 0) {
> + rc = errno;
> + }
> +
> + VIR_FREE(monitorfile);
> + return rc;
> +}
> +
> +static int
> +qemudFileReadMonitor(const char *dir,
> + const char *name,
> + char **path)
> +{
> + int rc;
> + FILE *file;
> + char *monitorfile = NULL;
> +
> + if (asprintf(&monitorfile, "%s/%s.monitor", dir, name) < 0) {
> + rc = ENOMEM;
> + goto cleanup;
> + }
> +
> + if (!(file = fopen(monitorfile, "r"))) {
> + rc = errno;
> + goto cleanup;
> + }
> +
> + if (fscanf(file, "%as", path) != 1) {
> + rc = EINVAL;
> + goto cleanup;
> + }
> +
> + if (fclose(file) < 0) {
> + rc = errno;
> + goto cleanup;
> + }
> +
> + rc = 0;
> +
> + cleanup:
> + VIR_FREE(monitorfile);
> + return rc;
> +}
If we re-parse the logfile to extract the monitiro PTY path, this is
unneccessary. If not, this can be simplied by just calling the
convenient virFileReadAll() method.
> +
> +static int
> +qemudFileDeleteMonitor(const char *dir,
> + const char *name)
> +{
> + int rc = 0;
> + char *pidfile = NULL;
> +
> + if (asprintf(&pidfile, "%s/%s.monitor", dir, name) < 0) {
> + rc = errno;
> + goto cleanup;
> + }
> +
> + if (unlink(pidfile) < 0 && errno != ENOENT)
> + rc = errno;
> +
> +cleanup:
> + VIR_FREE(pidfile);
> + return rc;
> +}
> +
> +
> +static int qemudOpenMonitor(virConnectPtr conn,
> + struct qemud_driver *driver,
> + virDomainObjPtr vm,
> + const char *monitor,
> + int reconnect);
> +/**
> + * qemudReconnectVMs
> + *
> + * Reconnect running vms to the daemon process
> + */
> +static int
> +qemudReconnectVMs(struct qemud_driver *driver)
> +{
> + virDomainObjPtr vm = driver->domains;
> +
> + while (vm) {
> + virDomainDefPtr tmp;
> + char *config = NULL;
> + char *monitor = NULL;
> + int rc;
> +
> + /* Read pid */
> + if ((rc = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0) {
> + virFileDeletePid(driver->stateDir, vm->def->name);
> + DEBUG("Found pid %d for '%s'", vm->pid, vm->def->name);
> + } else
> + goto next;
> +
> + if ((config = virDomainConfigFile(NULL,
> + driver->stateDir,
> + vm->def->name)) == NULL) {
> + qemudLog(QEMUD_ERR, _("Failed to read back domain config for %s\n"),
> + vm->def->name);
> + goto next;
> + }
> +
> + /* Try and load the config */
> + tmp = virDomainDefParseFile(NULL, driver->caps, config);
> + unlink(config);
> + if (tmp) {
> + vm->newDef = vm->def;
> + vm->def = tmp;
> + }
> +
> + /* read back monitor path */
> + if ((rc = qemudFileReadMonitor(driver->stateDir, vm->def->name, &monitor)) != 0) {
> + qemudLog(QEMUD_ERR, _("Failed to read back monitor path for %s: %s\n"),
> + vm->def->name, strerror(rc));
> + goto next;
> + }
> + qemudFileDeleteMonitor(driver->stateDir, vm->def->name);
> +
> + /* FIXME: reconnect stdout/stderr via /proc/pid/fd/ */
> +
> + if ((rc = qemudOpenMonitor(NULL, driver, vm, monitor, 1)) != 0) {
> + qemudLog(QEMUD_ERR, _("Failed to reconnect to monitor for %s: %d\n"),
> + vm->def->name, rc);
> + goto next;
> + }
> + vm->def->id = driver->nextvmid++;
The ID of a vm must never change for until it is rebooted. Simply
don't overwrite the existing vm->def->id that we jjust loaded off
disk. And instead adjust the nextvmid field, eg
if (vm->def->id >= driver->nextvmid)
driver->nextvmid = vm->def->id + 1;
> + vm->state = VIR_DOMAIN_RUNNING;
> +next:
> + VIR_FREE(config);
> + VIR_FREE(monitor);
> + vm = vm->next;
> + }
> + return 0;
> +}
> +
> /**
> * qemudStartup:
> *
> @@ -197,6 +372,10 @@ qemudStartup(void) {
>
> if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL)
> goto out_of_memory;
> +
> + if (asprintf (&qemu_driver->stateDir,
> + "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1)
> + goto out_of_memory;
> } else {
> if (!(pw = getpwuid(uid))) {
> qemudLog(QEMUD_ERR, _("Failed to find user record for uid '%d': %s\n"),
> @@ -210,6 +389,10 @@ qemudStartup(void) {
>
> if (asprintf (&base, "%s/.libvirt", pw->pw_dir) == -1)
> goto out_of_memory;
> +
> + if (asprintf (&qemu_driver->stateDir,
> + "%s/run/libvirt/qemu/", base) == -1)
> + goto out_of_memory;
> }
>
> /* Configuration paths are either ~/.libvirt/qemu/... (session) or
> @@ -250,6 +433,8 @@ qemudStartup(void) {
> qemudShutdown();
> return -1;
> }
> + qemudReconnectVMs(qemu_driver);
> +
> if (virNetworkLoadAllConfigs(NULL,
> &qemu_driver->networks,
> qemu_driver->networkConfigDir,
> @@ -329,6 +514,34 @@ qemudActive(void) {
> return 0;
> }
>
> +/**
> + * qemudSaveDomainState
> + *
> + * Save the full state of a running domain to statedir
> + *
> + * Returns 0 on success
> + */
> +static int
> +qemudSaveDomainState(struct qemud_driver * driver, virDomainObjPtr vm) {
> + int ret;
> +
> + if ((ret = virFileWritePid(driver->stateDir, vm->def->name, vm->pid)) != 0) {
> + qemudLog(QEMUD_ERR, _("Unable to safe pidfile for %s: %s\n"),
> + vm->def->name, strerror(ret));
> + return ret;
> + }
> + if ((ret = virDomainSaveConfig(NULL, driver->stateDir, vm->def))) {
> + qemudLog(QEMUD_ERR, _("Unable to save domain %s\n"), vm->def->name);
> + return ret;
> + }
> + if ((ret = qemudFileWriteMonitor(driver->stateDir, vm->def->name, vm->monitorpath)) != 0) {
> + qemudLog(QEMUD_ERR, _("Unable to monitor file for %s: %s\n"),
> + vm->def->name, strerror(ret));
> + return ret;
> + }
> + return 0;
> +}
> +
This will need to be called at time of VM creation, and the
XSL config will need updating whether live config changes.
> /**
> * qemudShutdown:
> *
> @@ -349,10 +562,14 @@ qemudShutdown(void) {
> while (vm) {
> virDomainObjPtr next = vm->next;
> if (virDomainIsActive(vm))
> +#if 1
> + qemudSaveDomainState(qemu_driver, vm);
> +#else
> qemudShutdownVMDaemon(NULL, qemu_driver, vm);
> if (!vm->persistent)
> virDomainRemoveInactive(&qemu_driver->domains,
> vm);
> +#endif
> vm = next;
> }
>
> @@ -389,6 +606,7 @@ qemudShutdown(void) {
> VIR_FREE(qemu_driver->networkConfigDir);
> VIR_FREE(qemu_driver->networkAutostartDir);
> VIR_FREE(qemu_driver->vncTLSx509certdir);
> + VIR_FREE(qemu_driver->stateDir);
>
> if (qemu_driver->brctl)
> brShutdown(qemu_driver->brctl);
> @@ -499,7 +717,7 @@ qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED,
> static int qemudOpenMonitor(virConnectPtr conn,
> struct qemud_driver *driver,
> virDomainObjPtr vm,
> - const char *monitor) {
> + const char *monitor, int reconnect) {
> int monfd;
> char buf[1024];
> int ret = -1;
> @@ -520,11 +738,26 @@ static int qemudOpenMonitor(virConnectPtr conn,
> goto error;
> }
>
> - ret = qemudReadMonitorOutput(conn,
> - driver, vm, monfd,
> - buf, sizeof(buf),
> - qemudCheckMonitorPrompt,
> - "monitor");
> + if (!reconnect) {
> + ret = qemudReadMonitorOutput(conn,
> + driver, vm, monfd,
> + buf, sizeof(buf),
> + qemudCheckMonitorPrompt,
> + "monitor");
> +
> + } else {
> + vm->monitor = monfd;
> + ret = 0;
> + }
> +
> + if (ret != 0)
> + goto error;
> +
> + if (!(vm->monitorpath = strdup(monitor))) {
> + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
> + "%s", _("failed to allocate space for monitor path"));
> + goto error;
> + }
>
> /* Keep monitor open upon success */
> if (ret == 0)
> @@ -623,7 +856,7 @@ qemudFindCharDevicePTYs(virConnectPtr conn,
> }
>
> /* Got them all, so now open the monitor console */
> - ret = qemudOpenMonitor(conn, driver, vm, monitor);
> + ret = qemudOpenMonitor(conn, driver, vm, monitor, 0);
>
> cleanup:
> VIR_FREE(monitor);
> @@ -966,12 +1199,11 @@ static int qemudStartVMDaemon(virConnectPtr conn,
>
> ret = virExec(conn, argv, NULL, &keepfd, &vm->pid,
> vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
> - VIR_EXEC_NONBLOCK);
> + VIR_EXEC_NONBLOCK | VIR_EXEC_SETSID);
> if (ret == 0) {
> vm->def->id = driver->nextvmid++;
> vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
> }
> -
> for (i = 0 ; argv[i] ; i++)
> VIR_FREE(argv[i]);
> VIR_FREE(argv);
> @@ -1037,7 +1269,9 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
>
> qemudLog(QEMUD_INFO, _("Shutting down VM '%s'\n"), vm->def->name);
>
> - kill(vm->pid, SIGTERM);
> + if (kill(vm->pid, SIGTERM) < 0)
> + qemudLog(QEMUD_ERROR, _("Failed to send SIGTERM to %s (%d): %s\n"),
> + vm->def->name, vm->pid, strerror(errno));
>
> qemudVMData(driver, vm, vm->stdout_fd);
> qemudVMData(driver, vm, vm->stderr_fd);
> @@ -1057,13 +1291,9 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
> vm->stderr_fd = -1;
> vm->monitor = -1;
>
> - if (waitpid(vm->pid, NULL, WNOHANG) != vm->pid) {
> - kill(vm->pid, SIGKILL);
> - if (waitpid(vm->pid, NULL, 0) != vm->pid) {
> - qemudLog(QEMUD_WARN,
> - "%s", _("Got unexpected pid, damn\n"));
> - }
> - }
> + /* shut if off for sure */
> + kill(vm->pid, SIGKILL);
> + virFileDeletePid(driver->stateDir, vm->def->name);
>
> vm->pid = -1;
> vm->def->id = -1;
> diff --git a/src/util.c b/src/util.c
> index ca14be1..442c810 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -299,14 +299,16 @@ virExec(virConnectPtr conn,
> !FD_ISSET(i, keepfd)))
> close(i);
>
> - if (flags & VIR_EXEC_DAEMON) {
> + if (flags & VIR_EXEC_DAEMON || flags & VIR_EXEC_SETSID) {
> if (setsid() < 0) {
> ReportError(conn, VIR_ERR_INTERNAL_ERROR,
> _("cannot become session leader: %s"),
> strerror(errno));
> _exit(1);
> }
> + }
>
> + if (flags & VIR_EXEC_DAEMON) {
> if (chdir("/") < 0) {
> ReportError(conn, VIR_ERR_INTERNAL_ERROR,
> _("cannot change to root directory: %s"),
> diff --git a/src/util.h b/src/util.h
> index 093ef46..8fbe2cd 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -32,6 +32,7 @@ enum {
> VIR_EXEC_NONE = 0,
> VIR_EXEC_NONBLOCK = (1 << 0),
> VIR_EXEC_DAEMON = (1 << 1),
> + VIR_EXEC_SETSID = (1 << 2),
> };
Shouldn't we simply be starting all the QEMU VMs with VIR_EXEC_DAEMON
so they totally disassociate themselves from libvirtd right away. They
will be disassociated anyway if the libvirtd is ever restarted, so best
to daemonize from time they are started, to avoid any surprising changes
in behaviour
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 10 months
[libvirt] [PATCH 1/2] Java bindings for domain events
by David Lively
The attached patch (against libvirt-java) implements Java bindings for
libvirt domain events. This version provides a libvirt EventImpl
running in its own Java Thread, and provides per-Connect synchronization
that makes using the bindings thread-safe. (Note the Domain, Network,
StoragePool, and StorageVol methods also synchronize on their Connect
object, as required by libvirt. I have similar changes for
NodeDevice.java that need to be made when that code is checked in.)
This version of the patch also implements and uses an enum class
(DomainEvent.Type), as suggested by Tóth István.
IMPORTANT: THIS PATCH WILL BREAK THINGS UNLESS THE NEXT [PATCH 2/2] IS
APPLIED TO libvirt FIRST. Also, libvirt must be compiled WITH_PTHREADS
for Java events to work.
Dave
15 years, 11 months
[libvirt] [PATCH 2/2]: Call udevsettle in the appropriate places
by Chris Lalancette
Instead of relying solely on polling for /dev devices to appear in libvirt, we
really should be synchronizing against udev. This is generally done by a call
to udevsettle, which is exactly what this patch implements for the storage
backends that are likely to create new /dev nodes. I believe I've read that
even after udevsettle, you are not guaranteed that devices are all the way
created, so we still need the polling in the rest of the sources, but this
should give us a much better chance of things existing as we expect.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
Index: src/storage_backend.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.29
diff -u -r1.29 storage_backend.c
--- a/src/storage_backend.c 17 Nov 2008 11:19:33 -0000 1.29
+++ b/src/storage_backend.c 26 Nov 2008 12:40:19 -0000
@@ -270,6 +270,20 @@
return 0;
}
+void virStorageBackendWaitForDevices(virConnectPtr conn)
+{
+ const char *const settleprog[] = { UDEVSETTLE, NULL };
+ int exitstatus;
+
+ /*
+ * NOTE: we ignore errors here; this is just to make sure that any device
+ * nodes that are being created finish before we try to
+ * scan them. If this fails (udevsettle exits badly, or doesn't exist
+ * at all), then we still have a fallback mechanism anyway.
+ */
+ virRun(conn, settleprog, &exitstatus);
+}
+
/*
* Given a volume path directly in /dev/XXX, iterate over the
* entries in the directory pool->def->target.path and find the
Index: src/storage_backend.h
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.13
diff -u -r1.13 storage_backend.h
--- a/src/storage_backend.h 17 Nov 2008 11:19:33 -0000 1.13
+++ b/src/storage_backend.h 26 Nov 2008 12:40:19 -0000
@@ -69,6 +69,8 @@
int fd,
int withCapacity);
+void virStorageBackendWaitForDevices(virConnectPtr conn);
+
char *virStorageBackendStablePath(virConnectPtr conn,
virStoragePoolObjPtr pool,
const char *devpath);
Index: src/storage_backend_logical.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_logical.c,v
retrieving revision 1.26
diff -u -r1.26 storage_backend_logical.c
--- a/src/storage_backend_logical.c 17 Nov 2008 11:19:33 -0000 1.26
+++ b/src/storage_backend_logical.c 26 Nov 2008 12:40:19 -0000
@@ -470,6 +470,8 @@
};
int exitstatus;
+ virStorageBackendWaitForDevices(conn);
+
/* Get list of all logical volumes */
if (virStorageBackendLogicalFindLVs(conn, pool, NULL) < 0) {
virStoragePoolObjClearVols(pool);
Index: src/storage_backend_disk.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_disk.c,v
retrieving revision 1.20
diff -u -r1.20 storage_backend_disk.c
--- a/src/storage_backend_disk.c 17 Nov 2008 11:19:33 -0000 1.20
+++ b/src/storage_backend_disk.c 26 Nov 2008 12:40:19 -0000
@@ -262,6 +262,8 @@
VIR_FREE(pool->def->source.devices[0].freeExtents);
pool->def->source.devices[0].nfreeExtent = 0;
+ virStorageBackendWaitForDevices(conn);
+
return virStorageBackendDiskReadPartitions(conn, pool, NULL);
}
Index: src/storage_backend_iscsi.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend_iscsi.c,v
retrieving revision 1.18
diff -u -r1.18 storage_backend_iscsi.c
--- a/src/storage_backend_iscsi.c 17 Nov 2008 11:19:33 -0000 1.18
+++ b/src/storage_backend_iscsi.c 26 Nov 2008 12:40:19 -0000
@@ -603,6 +603,8 @@
pool->def->allocation = pool->def->capacity = pool->def->available = 0;
+ virStorageBackendWaitForDevices(conn);
+
if ((session = virStorageBackendISCSISession(conn, pool)) == NULL)
goto cleanup;
if (virStorageBackendISCSIRescanLUNs(conn, pool, session) < 0)
Index: configure.in
===================================================================
RCS file: /data/cvs/libvirt/configure.in,v
retrieving revision 1.187
diff -u -r1.187 configure.in
--- a/configure.in 25 Nov 2008 15:48:11 -0000 1.187
+++ b/configure.in 26 Nov 2008 12:40:19 -0000
@@ -115,11 +115,15 @@
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_PATH_PROG([BRCTL], [brctl], [brctl],
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
+AC_PATH_PROG([UDEVSETTLE], [udevsettle], [udevsettle],
+ [/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"],
[Location or name of the dnsmasq program])
AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"],
[Location or name of the brctl program (see bridge-utils)])
+AC_DEFINE_UNQUOTED([UDEVSETTLE],["$UDEVSETTLE"],
+ [Location or name of the udevsettle program (see udev)])
dnl Specific dir for HTML output ?
AC_ARG_WITH([html-dir], [AC_HELP_STRING([--with-html-dir=path],
15 years, 11 months
[libvirt] Concerning umlInotifyEvent
by Ron Yorston
At the top of umlInotifyEvent in uml_driver.c there is this test:
if (watch != driver->inotifyWatch)
return;
This doesn't seem to be correct. I have to comment out the test to get
libvirtd to work with UML.
Ron
15 years, 11 months
[libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
by Gerd von Egidy
Hi,
I'm using libvirt 0.5.0 to manage my kvm domains. Everything works fine except
autostarting vms which use a network device not directly bridged to hw. I
have the following net defined (and set to autostart):
<network>
<name>local</name>
<uuid>7ec7a457-0b8e-4670-3b4a-2d0914288daa</uuid>
<bridge stp='on' forwardDelay='0' />
<ip address='192.168.100.1' netmask='255.255.255.0'>
<dhcp>
<range start='192.168.100.128' end='192.168.100.254' />
</dhcp>
</ip>
</network>
When a vm using this net is autostarted I get the following error:
libvir: error : invalid connection pointer in virNetworkLookupByName
libvir: QEMU error : internal error Network 'local' not found
Failed to autostart VM 'testlocalnet': internal error Network 'local' not
found
I took a dive into the code and found two problems causing this:
1. initialization order:
qemuStateDriver is registered (and thus initialized) before
networkStateDriver. This means the network is not up yet when qemu does it's
autostart.
I could fix this with the attached patch.
2. missing virConnectPtr during autostart of the vm:
in src/qemu_driver.c:145, qemudAutostartConfigs() you can find this call:
int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
This means virConnectPtr conn is NULL within qemudStartVMDaemon().
Following the calls:
qemudStartVMDaemon()
-> qemudBuildCommandLine()
-> qemudNetworkIfaceConnect()
-> virNetworkLookupByName()
virNetworkLookupByName bails out if called with invalid virConnectPtr. This
means starting this vm fails.
Currently I'm not familiar enough with the libvirt structures to come up with
an idea how to fix this.
Could anybody with a deeper knowledge of the code take a look at this? Thank
you very much.
Kind regards,
Gerd
diff -r -u libvirt-0.5.0.orig/qemud/qemud.c libvirt-0.5.0/qemud/qemud.c
--- libvirt-0.5.0.orig/qemud/qemud.c 2008-11-21 13:47:32.000000000 +0100
+++ libvirt-0.5.0/qemud/qemud.c 2008-11-30 21:27:06.000000000 +0100
@@ -761,22 +761,13 @@
* If they try to use a open a connection for a module that
* is not loaded they'll get a suitable error at that point
*/
- virDriverLoadModule("qemu");
- virDriverLoadModule("lxc");
- virDriverLoadModule("uml");
virDriverLoadModule("network");
virDriverLoadModule("storage");
virDriverLoadModule("nodedev");
+ virDriverLoadModule("qemu");
+ virDriverLoadModule("lxc");
+ virDriverLoadModule("uml");
#else
-#ifdef WITH_QEMU
- qemuRegister();
-#endif
-#ifdef WITH_LXC
- lxcRegister();
-#endif
-#ifdef WITH_UML
- umlRegister();
-#endif
#ifdef WITH_NETWORK
networkRegister();
#endif
@@ -786,6 +777,15 @@
#if defined(HAVE_HAL) || defined(HAVE_DEVKIT)
nodedevRegister();
#endif
+#ifdef WITH_QEMU
+ qemuRegister();
+#endif
+#ifdef WITH_LXC
+ lxcRegister();
+#endif
+#ifdef WITH_UML
+ umlRegister();
+#endif
#endif
virEventRegisterImpl(virEventAddHandleImpl,
--
Address (better: trap) for people I really don't want to get mail from:
james(a)cactusamerica.com
15 years, 11 months
[libvirt] Memory leak in xen_inotify.c
by Niraj Tolia
Hi,
It looks like xenInotifyOpen() in xen_inotify.c has an opendir() call.
However, as there is no corresponding closedir() and opendir() uses
malloc internally, valgrind reports this as a memory leak. Adding a
closedir() to the end of this function should fix this.
Cheers,
Niraj
15 years, 11 months
[libvirt] libvirt + xen 3.2.1 oddities
by Guido Günther
Hi,
I just ran across these oddities when using a bit more libvirt+xen:
1.) virsh setmaxmem:
On a running domain:
# virsh setmaxmem domain 256000
completes but virsh dumpxml as well as the config.sxp still shows the
old amount of memory. Looks as the set_maxmem hypercall simply gets
ignored. xm mem-max works as expected. Smells like a bug in the ioctl?
2.) virsh list:
Sometimes (didn't find a pattern yet) when shutting down a running
domains and restarting it I'm seeing:
Id Name State
----------------------------------
0 Domain-0 running
2 foo idle
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
7 bar idle
Note that the number of errors the corresponds to the number of
shutdowns. VirXen_getdomaininfolist returns 7 in the above case.
virDomainLookupByID later on fails for these "additional" domains.
3.) virsh list: Duplicate domains:
Id Name State
----------------------------------
0 Domain-0 running
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
14 bar no state
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
16 bar idle
Domain 14 can't be shut down (xm list only lists domain 16).
Could be a similar problem as the above.
This is all libvirt 0.4.6 (but the code looks very similar in gurrent
CVS) and xen-3.2.1 on Debian. The detected ABI is hypervisor call v2,
sys ver6 dom ver5. And from a quick glance at the libxen-dev package the
structs seem to match.
Cheers,
-- Guido
15 years, 11 months
[libvirt] PATCH: Fix XM driver handling of disks without source file
by Daniel P. Berrange
It is possible for disks to be listed without a source file against
them, eg a CDROM device with no media loaded. The XenD driver handles
this, but the XM driver incorrectly generates XML with a <source file=''/>
element instead of omitting the element entirely. This causes a bogus
SXEXPR to be sent to XenD when starting the domain. This patch does
three things
- Makes the generic domain_conf.c XML parser accept XML docs with
a bogus <source file=''/> and convert the source to NULL, instead
of passing along the empty string "". This protects against broken
apps
- Makes the XM driver correctly generate XML in the first place,
so omitting the <source> tag entirely. This is the root cause fix
- Adds a test case for the XM driver to validate handling of devices
with a source file
src/domain_conf.c | 8 +++
src/xm_internal.c | 70 ++++++++++++++++------------
tests/xmconfigdata/test-no-source-cdrom.cfg | 23 +++++++++
tests/xmconfigdata/test-no-source-cdrom.xml | 46 ++++++++++++++++++
tests/xmconfigtest.c | 1
5 files changed, 120 insertions(+), 28 deletions(-)
Daniel
Index: src/xm_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xm_internal.c,v
retrieving revision 1.102
diff -u -p -r1.102 xm_internal.c
--- src/xm_internal.c 25 Nov 2008 11:18:08 -0000 1.102
+++ src/xm_internal.c 27 Nov 2008 12:14:57 -0000
@@ -828,7 +828,7 @@ xenXMDomainConfigParse(virConnectPtr con
while (list) {
char *head;
char *offset;
- char *tmp, *tmp1;
+ char *tmp;
if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
goto skipdisk;
@@ -850,10 +850,15 @@ xenXMDomainConfigParse(virConnectPtr con
goto skipdisk;
if ((offset - head) >= (PATH_MAX-1))
goto skipdisk;
- if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0)
- goto no_memory;
- strncpy(disk->src, head, (offset - head));
- disk->src[(offset-head)] = '\0';
+
+ if (offset == head) {
+ disk->src = NULL; /* No source file given, eg CDROM with no media */
+ } else {
+ if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0)
+ goto no_memory;
+ strncpy(disk->src, head, (offset - head));
+ disk->src[(offset-head)] = '\0';
+ }
head = offset + 1;
/* Remove legacy ioemu: junk */
@@ -871,32 +876,41 @@ xenXMDomainConfigParse(virConnectPtr con
/* Extract source driver type */
- if (disk->src &&
- (tmp = strchr(disk->src, ':')) != NULL) {
- if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0)
- goto no_memory;
- strncpy(disk->driverName, disk->src, (tmp - disk->src));
- disk->driverName[tmp - disk->src] = '\0';
- } else {
- if (!(disk->driverName = strdup("phy")))
- goto no_memory;
- tmp = disk->src;
- }
+ if (disk->src) {
+ /* The main type phy:, file:, tap: ... */
+ if ((tmp = strchr(disk->src, ':')) != NULL) {
+ if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0)
+ goto no_memory;
+ strncpy(disk->driverName, disk->src, (tmp - disk->src));
+ disk->driverName[tmp - disk->src] = '\0';
- /* And the source driver sub-type */
- if (STRPREFIX(disk->driverName, "tap")) {
- if (!(tmp1 = strchr(tmp+1, ':')) || !tmp1[0])
- goto skipdisk;
- if (VIR_ALLOC_N(disk->driverType, (tmp1-(tmp+1))) < 0)
- goto no_memory;
- strncpy(disk->driverType, tmp+1, (tmp1-(tmp+1)));
- memmove(disk->src, disk->src+(tmp1-disk->src)+1, strlen(disk->src)-(tmp1-disk->src));
- } else {
- disk->driverType = NULL;
- if (disk->src[0] && tmp)
- memmove(disk->src, disk->src+(tmp-disk->src)+1, strlen(disk->src)-(tmp-disk->src));
+ /* Strip the prefix we found off the source file name */
+ memmove(disk->src, disk->src+(tmp-disk->src)+1,
+ strlen(disk->src)-(tmp-disk->src));
+ }
+
+ /* And the sub-type for tap:XXX: type */
+ if (disk->driverName &&
+ STREQ(disk->driverName, "tap")) {
+ if (!(tmp = strchr(disk->src, ':')))
+ goto skipdisk;
+ if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0)
+ goto no_memory;
+ strncpy(disk->driverType, disk->src, (tmp - disk->src));
+ disk->driverType[tmp - disk->src] = '\0';
+
+ /* Strip the prefix we found off the source file name */
+ memmove(disk->src, disk->src+(tmp-disk->src)+1,
+ strlen(disk->src)-(tmp-disk->src));
+ }
}
+ /* No source, or driver name, so fix to phy: */
+ if (!disk->driverName &&
+ !(disk->driverName = strdup("phy")))
+ goto no_memory;
+
+
/* phy: type indicates a block device */
disk->type = STREQ(disk->driverName, "phy") ?
VIR_DOMAIN_DISK_TYPE_BLOCK : VIR_DOMAIN_DISK_TYPE_FILE;
Index: src/domain_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/domain_conf.c,v
retrieving revision 1.39
diff -u -p -r1.39 domain_conf.c
--- src/domain_conf.c 21 Nov 2008 11:42:51 -0000 1.39
+++ src/domain_conf.c 27 Nov 2008 12:15:06 -0000
@@ -546,6 +546,14 @@ virDomainDiskDefParseXML(virConnectPtr c
source = virXMLPropString(cur, "file");
else
source = virXMLPropString(cur, "dev");
+
+ /* People sometimes pass a bogus '' source path
+ when they mean to omit the source element
+ completely. eg CDROM without media. This is
+ just a little compatability check to help
+ those broken apps */
+ if (source && STREQ(source, ""))
+ VIR_FREE(source);
} else if ((target == NULL) &&
(xmlStrEqual(cur->name, BAD_CAST "target"))) {
target = virXMLPropString(cur, "dev");
Index: tests/xmconfigtest.c
===================================================================
RCS file: /data/cvs/libvirt/tests/xmconfigtest.c,v
retrieving revision 1.24
diff -u -p -r1.24 xmconfigtest.c
--- tests/xmconfigtest.c 24 Nov 2008 19:23:39 -0000 1.24
+++ tests/xmconfigtest.c 27 Nov 2008 12:15:13 -0000
@@ -229,6 +229,7 @@ mymain(int argc, char **argv)
DO_TEST("fullvirt-sound", 2);
DO_TEST("escape-paths", 2);
+ DO_TEST("no-source-cdrom", 2);
virCapabilitiesFree(caps);
Index: tests/xmconfigdata/test-no-source-cdrom.cfg
===================================================================
RCS file: tests/xmconfigdata/test-no-source-cdrom.cfg
diff -N tests/xmconfigdata/test-no-source-cdrom.cfg
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/xmconfigdata/test-no-source-cdrom.cfg 27 Nov 2008 12:15:27 -0000
@@ -0,0 +1,23 @@
+name = "test"
+uuid = "cc2315e7-d26a-307a-438c-6d188ec4c09c"
+maxmem = 382
+memory = 350
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader"
+boot = "c"
+pae = 1
+acpi = 1
+apic = 1
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "destroy"
+on_crash = "destroy"
+device_model = "/usr/lib/xen/bin/qemu-dm"
+sdl = 0
+vnc = 1
+vncunused = 1
+disk = [ "phy:/dev/sda8,hda,w", ",hdc:cdrom,r" ]
+vif = [ "mac=00:16:3e:0a:7b:39,bridge=xenbr0,type=ioemu" ]
+parallel = "none"
+serial = "pty"
Index: tests/xmconfigdata/test-no-source-cdrom.xml
===================================================================
RCS file: tests/xmconfigdata/test-no-source-cdrom.xml
diff -N tests/xmconfigdata/test-no-source-cdrom.xml
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/xmconfigdata/test-no-source-cdrom.xml 27 Nov 2008 12:15:27 -0000
@@ -0,0 +1,46 @@
+<domain type='xen'>
+ <name>test</name>
+ <uuid>cc2315e7-d26a-307a-438c-6d188ec4c09c</uuid>
+ <memory>391168</memory>
+ <currentMemory>358400</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='xenfv'>hvm</type>
+ <loader>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+ <disk type='block' device='disk'>
+ <driver name='phy'/>
+ <source dev='/dev/sda8'/>
+ <target dev='hda' bus='ide'/>
+ </disk>
+ <disk type='block' device='cdrom'>
+ <driver name='phy'/>
+ <target dev='hdc' bus='ide'/>
+ <readonly/>
+ </disk>
+ <interface type='bridge'>
+ <mac address='00:16:3e:0a:7b:39'/>
+ <source bridge='xenbr0'/>
+ </interface>
+ <serial type='pty'>
+ <target port='0'/>
+ </serial>
+ <console type='pty'>
+ <target port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'/>
+ </devices>
+</domain>
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 11 months
