[libvirt] [Patch 0/3]virsh: Patches for virsh logging
by Supriya Kannery
Virsh logging has some basic issues:
1. In code, magic numbers are used for logging rather than loglevel
variables.
2. Magic number "5" is used for logging which doesn't map to any
loglevel variable. Valid loglevel range is 0-4
3. Usage of loglevel variables doesn't align with that of libvirt
logging. In libvirt "DEBUG" loglevel is the superset and logs
messages at all other levels. Whereas in virsh, "ERROR" loglevel
behaves this way, which needs correction
4. virsh man page and code are inconsistent with respect to loglevels
Following patchset is to address the above mentioned issues.
1/3 - Avoid using magic numbers for logging
2/3 - Align log level usage to that of libvirt
3/3 - Update virsh manpage with related changes
tools/virsh.pod | 30 ++++++++++++
tools/virsh.c | 124 +++++++++++++++++++++++++++---------------------
2 files changed, 102 insertions(+), 52 deletions(-)
13 years, 5 months
[libvirt] [PATCHv2 00/27] flags cleanup
by Eric Blake
Addressing my review comments from round 1, and introducing a few
more goodies along the way. I've added some syntax checks to
make it easier to stick with this style in the future.
v1 was at https://www.redhat.com/archives/libvir-list/2011-July/msg00264.html,
with patches 1-5 already applied, and patches 6-20 of that series revamped
in this series as 5-19. Patches 1-4 and 20-27 in this series are new.
Eric Blake (27):
maint: exclude more files from syntax check
maint: print flags in hex during debug
build: also check qemu_protocol for on-the-wire stability
libvirt-qemu: use unsigned flags
util: reject unknown flags, and prefer unsigned flags
node_device: reject unknown flags
storage: reject unknown flags
esx: reject unknown flags
libxl: reject unknown flags
lxc: reject unknown flags
openvz: reject unknown flags
phyp: reject unknown flags
qemu: reject unknown flags
test: reject unknown flags
uml: reject unknown flags
vbox: reject unknown flags
vmware: reject unknown flags
xen: reject unknown flags
xenapi: reject unknown flags
virsh, daemon: prefer unsigned flags
node_device: avoid implicit int
python: prefer unsigned flags
conf: prefer unsigned flags
build: don't hand-roll cloexec code
conf: delete unused flags arguments
remote: prefer unsigned flags
build: add syntax check for proper flags use
cfg.mk | 49 +++++++++---
daemon/remote.c | 2 +-
python/libvirt-override.c | 6 +-
src/Makefile.am | 20 +++--
src/conf/cpu_conf.c | 6 +-
src/conf/cpu_conf.h | 6 +-
src/conf/domain_conf.c | 24 ++----
src/conf/node_device_conf.h | 58 +++++++-------
src/conf/storage_conf.c | 4 +-
src/datatypes.h | 4 +-
src/esx/esx_device_monitor.c | 4 +-
src/esx/esx_driver.c | 28 +++++--
src/esx/esx_interface_driver.c | 4 +-
src/esx/esx_network_driver.c | 4 +-
src/esx/esx_nwfilter_driver.c | 4 +-
src/esx/esx_secret_driver.c | 4 +-
src/esx/esx_storage_driver.c | 4 +-
src/fdstream.c | 28 +++---
src/fdstream.h | 6 +-
src/interface/netcf_driver.c | 16 +++-
src/libvirt-qemu.c | 5 +-
src/libxl/libxl_driver.c | 18 +++-
src/locking/lock_driver_nop.c | 13 ++--
src/locking/lock_driver_sanlock.c | 3 +-
src/locking/lock_manager.c | 18 +++--
src/lxc/lxc_container.c | 4 +-
src/lxc/lxc_driver.c | 12 ++-
src/network/bridge_driver.c | 9 ++-
src/node_device/node_device_driver.c | 18 +++-
src/node_device/node_device_hal.c | 4 +-
src/node_device/node_device_udev.c | 4 +-
src/nodeinfo.h | 6 +-
src/nwfilter/nwfilter_driver.c | 4 +-
src/openvz/openvz_driver.c | 9 ++-
src/phyp/phyp_driver.c | 12 ++-
src/qemu/qemu_domain.c | 21 +++---
src/qemu/qemu_domain.h | 4 +-
src/qemu/qemu_driver.c | 30 +++++--
src/qemu/qemu_migration.c | 32 ++++----
src/qemu/qemu_monitor.c | 10 +-
src/qemu_protocol-structs | 14 +++
src/remote/qemu_protocol.x | 4 +-
src/remote/remote_driver.c | 6 +-
src/rpc/virnetserverclient.c | 2 +-
src/secret/secret_driver.c | 17 +++-
src/storage/storage_backend.c | 12 ++-
src/storage/storage_backend_disk.c | 10 ++-
src/storage/storage_backend_fs.c | 26 +++++--
src/storage/storage_backend_iscsi.c | 4 +-
src/storage/storage_backend_logical.c | 18 +++-
src/storage/storage_driver.c | 45 ++++++++--
src/test/test_driver.c | 144 +++++++++++++++++++++++++-------
src/uml/uml_driver.c | 33 +++-----
src/util/bridge.c | 19 +---
src/util/command.c | 18 ++--
src/util/iohelper.c | 18 ++--
src/util/logging.c | 13 +++-
src/util/logging.h | 8 +-
src/util/util.c | 14 ++--
src/vbox/vbox_driver.c | 5 +-
src/vbox/vbox_tmpl.c | 44 ++++++++--
src/vmware/vmware_driver.c | 17 +++-
src/xen/xen_driver.c | 12 ++-
src/xen/xen_hypervisor.c | 8 ++-
src/xen/xen_inotify.c | 4 +-
src/xen/xend_internal.c | 23 ++++--
src/xen/xend_internal.h | 3 +-
src/xen/xm_internal.c | 11 ++-
src/xen/xm_internal.h | 2 +-
src/xen/xs_internal.c | 12 ++-
src/xenapi/xenapi_driver.c | 13 +++-
tools/virsh.c | 2 +-
72 files changed, 731 insertions(+), 367 deletions(-)
create mode 100644 src/qemu_protocol-structs
--
1.7.4.4
13 years, 5 months
[libvirt] [PATCH 0/3] child process cleanups
by Eric Blake
In preparing for making iohelper useful with O_DIRECT fds, I
noticed a resource leak on failure in fdstreams. Further
investigation found that most clients of virCommandRunAsync
passed a NULL pointer, and the only one that didn't was
risking closing an unrelated process under a pid reuse scenario.
Eric Blake (3):
command: introduce virPidWait, virPidAbort
fdstream: avoid child process leak on error
virnetsocket: use new API for uniform child cleanup
docs/internals/command.html.in | 17 +++++
src/fdstream.c | 8 +--
src/libvirt_private.syms | 2 +
src/rpc/virnetsocket.c | 13 +----
src/util/command.c | 140 ++++++++++++++++++++++++++++------------
src/util/command.h | 28 ++++++++
6 files changed, 148 insertions(+), 60 deletions(-)
--
1.7.4.4
13 years, 5 months
[libvirt] [PATCH] update apparmor security driver for new udev paths
by Jamie Strandboge
In the Ubuntu development release we recently got a new udev that
moves /var/run to /run, /var/lock to /run/lock and /dev/shm to /run/shm.
This change in udev requires updating the apparmor security driver in
libvirt[1].
Attached is a patch that:
* adjusts src/security/virt-aa-helper.c to allow both
LOCALSTATEDIR/run/libvirt/**/%s.pid and /run/libvirt/**/%s.pid. While
the profile is not as precise, LOCALSTATEDIR/run/ is typically a symlink
to /run/ anyway, so there is no additional access (remember that
apparmor resolves symlinks, which is why this is still required even
if /var/run points to /run).
* adjusts example/apparmor/libvirt-qemu paths for /dev/shm
[1]https://launchpad.net/bugs/810270
--
Jamie Strandboge | http://www.canonical.com
13 years, 5 months
[libvirt] [PATCH] xenapi: Improve error reporting in xenapiOpen
by Matthias Bolte
Use better suited error code and avoid NULL in error messsage
as *privP->session->error_description can be NULL.
---
src/xenapi/xenapi_driver.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index aa616ca..98838e6 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -106,7 +106,7 @@ xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth,
}
if (conn->uri->server == NULL) {
- xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED,
+ xenapiSessionErrorHandler(conn, VIR_ERR_INVALID_ARG,
_("Server name not in URI"));
goto error;
}
@@ -194,7 +194,9 @@ xenapiOpen (virConnectPtr conn, virConnectAuthPtr auth,
}
xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED,
- *privP->session->error_description);
+ *privP->session->error_description != NULL ?
+ *privP->session->error_description :
+ _("unknown error"));
error:
VIR_FREE(username);
--
1.7.4.1
13 years, 5 months
[libvirt] [PATCH] build: check for virnetprotocol on-the-wire stability
by Eric Blake
Similar to the recent qemu_protocol-structs addition.
* src/virnetprotocol-structs: New file.
* src/Makefile.am (%_protocol-structs): Factor body...
(PDWTAGS): ...into new helper macro.
(virnetprotocol-structs): New rule.
(PROTOCOL_STRUCTS): Add virnetprotocol-structs.
---
In reply to:
https://www.redhat.com/archives/libvir-list/2011-July/msg00602.html
src/Makefile.am | 24 +++++++++++++++---------
src/virnetprotocol-structs | 31 +++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+), 9 deletions(-)
create mode 100644 src/virnetprotocol-structs
diff --git a/src/Makefile.am b/src/Makefile.am
index 39f0cf8..d19d1ca 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -212,18 +212,12 @@ EXTRA_DIST += $(REMOTE_DRIVER_PROTOCOL) \
r1 = (?:/\* \d+ \*/\n)?
r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/
-PROTOCOL_STRUCTS = \
- $(srcdir)/remote_protocol-structs \
- $(srcdir)/qemu_protocol-structs
-if WITH_REMOTE
-# The .o file that pdwtags parses is created as a side effect of running
-# libtool; but from make's perspective we depend on the .lo file.
-%_protocol-structs: libvirt_driver_remote_la-%_protocol.lo
+PDWTAGS = \
$(AM_V_GEN)if (pdwtags --help) > /dev/null 2>&1; then \
pdwtags --verbose $(<:.lo=.$(OBJEXT)) \
| perl -0777 -n \
-e 'foreach my $$p (split m!\n\n$(r1)$(r2)\n!) {' \
- -e ' if ($$p =~ /^struct (remote|qemu)_/) {' \
+ -e ' if ($$p =~ /^struct (remote_|qemu_|virNet)/) {' \
-e ' $$p =~ s!\t*/\*.*?\*/!!sg;' \
-e ' $$p =~ s!\s+\n!\n!sg;' \
-e ' $$p =~ s!\s+$$!!;' \
@@ -250,10 +244,22 @@ if WITH_REMOTE
echo 'WARNING: you lack pdwtags; skipping the $@ test' >&2; \
echo 'WARNING: install the dwarves package to get pdwtags' >&2; \
fi
+
+PROTOCOL_STRUCTS = \
+ $(srcdir)/remote_protocol-structs \
+ $(srcdir)/qemu_protocol-structs \
+ $(srcdir)/virnetprotocol-structs
+if WITH_REMOTE
+# The .o file that pdwtags parses is created as a side effect of running
+# libtool; but from make's perspective we depend on the .lo file.
+$(srcdir)/%_protocol-structs: libvirt_driver_remote_la-%_protocol.lo
+ $(PDWTAGS)
+$(srcdir)/virnetprotocol-structs: libvirt_net_rpc_la-virnetprotocol.lo
+ $(PDWTAGS)
else !WITH_REMOTE
# These generated files must live in git, because they cannot be re-generated
# when configured --without-remote.
-$(srcdir)/%_protocol-structs:
+$(PROTOCOL_STRUCTS):
endif
EXTRA_DIST += $(PROTOCOL_STRUCTS)
check-local: $(PROTOCOL_STRUCTS)
diff --git a/src/virnetprotocol-structs b/src/virnetprotocol-structs
new file mode 100644
index 0000000..1ee2c6d
--- /dev/null
+++ b/src/virnetprotocol-structs
@@ -0,0 +1,31 @@
+/* -*- c -*- */
+struct virNetMessageHeader {
+ u_int prog;
+ u_int vers;
+ int proc;
+ virNetMessageType type;
+ u_int serial;
+ virNetMessageStatus status;
+};
+struct virNetMessageNonnullDomain {
+ virNetMessageNonnullString name;
+ virNetMessageUUID uuid;
+ int id;
+};
+struct virNetMessageNonnullNetwork {
+ virNetMessageNonnullString name;
+ virNetMessageUUID uuid;
+};
+struct virNetMessageError {
+ int code;
+ int domain;
+ virNetMessageString message;
+ int level;
+ virNetMessageDomain dom;
+ virNetMessageString str1;
+ virNetMessageString str2;
+ virNetMessageString str3;
+ int int1;
+ int int2;
+ virNetMessageNetwork net;
+};
--
1.7.4.4
13 years, 5 months
[libvirt] [PATCH] Fix rpm build with sanlock and without QEmu
by Daniel Veillard
The qemu-sanlock.conf file is not installed in this case
Pushed under build breaker rules
Daniel
diff --git a/libvirt.spec.in b/libvirt.spec.in
index bf220f3..230237e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1031,7 +1031,9 @@ fi
%if %{with_sanlock}
%files lock-sanlock
%defattr(-, root, root)
+%if %{with_qemu}
%config(noreplace) %{_sysconfdir}/libvirt/qemu-sanlock.conf
+%endif
%attr(0755, root, root) %{_libdir}/libvirt/lock-driver/sanlock.so
%{_datadir}/augeas/lenses/libvirt_sanlock.aug
%{_datadir}/augeas/lenses/tests/test_libvirt_sanlock.aug
--
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/
13 years, 5 months
[libvirt] [PATCH v3] storage: Avoid memory leak on metadata fetching
by Michal Privoznik
Getting metadata on storage allocates a memory (path) which need to
be freed after use otherwise it gets leaked. This means after use of
virStorageFileGetMetadataFromFD or virStorageFileGetMetadata one
must call virStorageFileFreeMetadata to free it. This function frees
structure internals and structure itself.
---
diff to v2:
-Jrika's & Eric's review taken in
diff to v1:
-Eric's review suggestions taken in
cfg.mk | 1 +
src/conf/domain_conf.c | 18 +++++++++---
src/libvirt_private.syms | 1 +
src/storage/storage_backend_fs.c | 54 +++++++++++++++++++++----------------
src/util/storage_file.c | 18 ++++++++++++
src/util/storage_file.h | 2 +
6 files changed, 66 insertions(+), 28 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index ffaca85..a88a6a9 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -156,6 +156,7 @@ useless_free_options = \
--name=virSecretDefFree \
--name=virStorageEncryptionFree \
--name=virStorageEncryptionSecretFree \
+ --name=virStorageFileFreeMetadata \
--name=virStoragePoolDefFree \
--name=virStoragePoolObjFree \
--name=virStoragePoolSourceFree \
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 39e59b9..a895e98 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11055,10 +11055,16 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
int ret = -1;
size_t depth = 0;
char *nextpath = NULL;
+ virStorageFileMetadata *meta;
if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
return 0;
+ if (VIR_ALLOC(meta) < 0) {
+ virReportOOMError();
+ return ret;
+ }
+
if (disk->driverType) {
const char *formatStr = disk->driverType;
if (STREQ(formatStr, "aio"))
@@ -11084,7 +11090,6 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
paths = virHashCreate(5, NULL);
do {
- virStorageFileMetadata meta;
const char *path = nextpath ? nextpath : disk->src;
int fd;
@@ -11112,7 +11117,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
- if (virStorageFileGetMetadataFromFD(path, fd, format, &meta) < 0) {
+ if (virStorageFileGetMetadataFromFD(path, fd, format, meta) < 0) {
VIR_FORCE_CLOSE(fd);
goto cleanup;
}
@@ -11126,16 +11131,18 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
goto cleanup;
depth++;
- nextpath = meta.backingStore;
+ VIR_FREE(nextpath);
+ nextpath = meta->backingStore;
+ meta->backingStore = NULL;
/* Stop iterating if we reach a non-file backing store */
- if (nextpath && !meta.backingStoreIsFile) {
+ if (nextpath && !meta->backingStoreIsFile) {
VIR_DEBUG("Stopping iteration on non-file backing store: %s",
nextpath);
break;
}
- format = meta.backingStoreFormat;
+ format = meta->backingStoreFormat;
if (format == VIR_STORAGE_FILE_AUTO &&
!allowProbing)
@@ -11151,6 +11158,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
cleanup:
virHashFree(paths);
VIR_FREE(nextpath);
+ virStorageFileFreeMetadata(meta);
return ret;
}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3237d18..e06c7fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -940,6 +940,7 @@ virStorageGenerateQcowPassphrase;
# storage_file.h
virStorageFileFormatTypeFromString;
virStorageFileFormatTypeToString;
+virStorageFileFreeMetadata;
virStorageFileGetMetadata;
virStorageFileGetMetadataFromFD;
virStorageFileIsSharedFS;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index b30e01e..8d6f76d 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,8 +61,14 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
unsigned long long *capacity,
virStorageEncryptionPtr *encryption)
{
- int fd, ret;
- virStorageFileMetadata meta;
+ int fd = -1;
+ int ret = -1;
+ virStorageFileMetadata *meta;
+
+ if (VIR_ALLOC(meta) < 0) {
+ virReportOOMError();
+ return ret;
+ }
*backingStore = NULL;
*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
@@ -71,36 +77,33 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
if ((ret = virStorageBackendVolOpenCheckMode(target->path,
VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0)
- return ret; /* Take care to propagate ret, it is not always -1 */
+ goto error; /* Take care to propagate ret, it is not always -1 */
fd = ret;
if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
allocation,
capacity)) < 0) {
- VIR_FORCE_CLOSE(fd);
- return ret;
+ goto error;
}
- memset(&meta, 0, sizeof(meta));
-
if ((target->format = virStorageFileProbeFormatFromFD(target->path, fd)) < 0) {
- VIR_FORCE_CLOSE(fd);
- return -1;
+ ret = -1;
+ goto error;
}
if (virStorageFileGetMetadataFromFD(target->path, fd,
target->format,
- &meta) < 0) {
- VIR_FORCE_CLOSE(fd);
- return -1;
+ meta) < 0) {
+ ret = -1;
+ goto error;
}
VIR_FORCE_CLOSE(fd);
- if (meta.backingStore) {
- *backingStore = meta.backingStore;
- meta.backingStore = NULL;
- if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
+ if (meta->backingStore) {
+ *backingStore = meta->backingStore;
+ meta->backingStore = NULL;
+ if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
if ((ret = virStorageFileProbeFormat(*backingStore)) < 0) {
/* If the backing file is currently unavailable, only log an error,
* but continue. Returning -1 here would disable the whole storage
@@ -114,18 +117,17 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
ret = 0;
}
} else {
- *backingStoreFormat = meta.backingStoreFormat;
+ *backingStoreFormat = meta->backingStoreFormat;
ret = 0;
}
} else {
- VIR_FREE(meta.backingStore);
ret = 0;
}
- if (capacity && meta.capacity)
- *capacity = meta.capacity;
+ if (capacity && meta->capacity)
+ *capacity = meta->capacity;
- if (encryption != NULL && meta.encrypted) {
+ if (encryption != NULL && meta->encrypted) {
if (VIR_ALLOC(*encryption) < 0) {
virReportOOMError();
goto cleanup;
@@ -147,11 +149,17 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
*/
}
+ virStorageFileFreeMetadata(meta);
+
return ret;
+error:
+ VIR_FORCE_CLOSE(fd);
+
cleanup:
- VIR_FREE(*backingStore);
- return -1;
+ virStorageFileFreeMetadata(meta);
+ return ret;
+
}
#if WITH_STORAGE_FS
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 06cabc8..d4460d8 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -819,6 +819,8 @@ virStorageFileProbeFormat(const char *path)
* it indicates the image didn't specify an explicit format for its
* backing store. Callers are advised against probing for the
* backing store format in this case.
+ *
+ * Caller MUST free @meta after use via virStorageFileFreeMetadata.
*/
int
virStorageFileGetMetadataFromFD(const char *path,
@@ -892,6 +894,8 @@ cleanup:
* it indicates the image didn't specify an explicit format for its
* backing store. Callers are advised against probing for the
* backing store format in this case.
+ *
+ * Caller MUST free @meta after use via virStorageFileFreeMetadata.
*/
int
virStorageFileGetMetadata(const char *path,
@@ -912,6 +916,20 @@ virStorageFileGetMetadata(const char *path,
return ret;
}
+/**
+ * virStorageFileFreeMetadata:
+ *
+ * Free pointers in passed structure and structure itself.
+ */
+void
+virStorageFileFreeMetadata(virStorageFileMetadata *meta)
+{
+ if (!meta)
+ return;
+
+ VIR_FREE(meta->backingStore);
+ VIR_FREE(meta);
+}
#ifdef __linux__
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index f1bdd02..b8920d0 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -70,6 +70,8 @@ int virStorageFileGetMetadataFromFD(const char *path,
int format,
virStorageFileMetadata *meta);
+void virStorageFileFreeMetadata(virStorageFileMetadata *meta);
+
enum {
VIR_STORAGE_FILE_SHFS_NFS = (1 << 0),
VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1),
--
1.7.5.rc3
13 years, 5 months
[libvirt] [PATCH] qemu: Save domain status ASAP after creating qemu process
by Jiri Denemark
When creating new qemu process we saved domain status XML only after the
process was fully setup and running. In case libvirtd was killed before
the whole process finished, once libvirtd started again it didn't know
anything about the new process and we end up with an orphaned qemu
process. Let's save the domain status XML as soon as we know the PID so
that libvirtd can kill the process on restart.
---
src/qemu/qemu_process.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 52a73b8..d0085e0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2226,6 +2226,10 @@ qemuProcessUpdateState(struct qemud_driver *driver, virDomainObjPtr vm)
VIR_DEBUG("Domain %s was paused while its monitor was disconnected;"
" changing state to paused", vm->def->name);
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_UNKNOWN);
+ } else if (state == VIR_DOMAIN_SHUTOFF && running) {
+ VIR_DEBUG("Domain %s finished booting; changing state to running",
+ vm->def->name);
+ virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
}
return 0;
@@ -2349,6 +2353,12 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa
if (qemuProcessUpdateState(driver, obj) < 0)
goto error;
+ if (virDomainObjGetState(obj, NULL) == VIR_DOMAIN_SHUTOFF) {
+ VIR_DEBUG("Domain '%s' wasn't fully started yet, killing it",
+ obj->def->name);
+ goto error;
+ }
+
/* If upgrading from old libvirtd we won't have found any
* caps in the domain status, so re-query them
*/
@@ -2463,6 +2473,7 @@ int qemuProcessStart(virConnectPtr conn,
vm->def->id = driver->nextvmid++;
priv->fakeReboot = false;
+ virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN);
/* Run an early hook to set-up missing devices */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
@@ -2719,6 +2730,12 @@ int qemuProcessStart(virConnectPtr conn,
#endif
}
+ VIR_DEBUG("Writing early domain status to disk");
+ if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
VIR_DEBUG("Waiting for handshake from child");
if (virCommandHandshakeWait(cmd) < 0) {
ret = -1;
--
1.7.6
13 years, 5 months
[libvirt] [PATCH] qemu: Don't overwrite errors by closefd in error paths
by Jiri Denemark
When qemuMonitorCloseFileHandle is called in error path, we need to
preserve the original error since a possible further error when running
closefd monitor command is not very useful to users.
---
src/qemu/qemu_monitor.c | 34 +++++++++++++++++++++++++---------
src/qemu/qemu_monitor.h | 3 ++-
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index e593642..4c6c66f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1552,7 +1552,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon,
ret = qemuMonitorTextMigrate(mon, flags, "fd:migrate");
if (ret < 0) {
- if (qemuMonitorCloseFileHandle(mon, "migrate") < 0)
+ if (qemuMonitorCloseFileHandle(mon, "migrate", true) < 0)
VIR_WARN("failed to close migration handle");
}
@@ -1962,22 +1962,34 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
- const char *fdname)
+ const char *fdname,
+ bool preserveError)
{
- int ret;
+ int ret = -1;
+ virErrorPtr error = NULL;
+
VIR_DEBUG("mon=%p fdname=%s",
mon, fdname);
+ if (preserveError)
+ error = virSaveLastError();
+
if (!mon) {
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("monitor must not be NULL"));
- return -1;
+ goto cleanup;
}
if (mon->json)
ret = qemuMonitorJSONCloseFileHandle(mon, fdname);
else
ret = qemuMonitorTextCloseFileHandle(mon, fdname);
+
+cleanup:
+ if (error) {
+ virSetError(error);
+ virFreeError(error);
+ }
return ret;
}
@@ -2014,9 +2026,11 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon,
cleanup:
if (ret < 0) {
- if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0)
+ if (tapfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, tapfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'", tapfd_name);
- if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) < 0)
+ if (vhostfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, vhostfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'", vhostfd_name);
}
@@ -2078,9 +2092,11 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
cleanup:
if (ret < 0) {
- if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0)
+ if (tapfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, tapfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'", tapfd_name);
- if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) < 0)
+ if (vhostfd >= 0 &&
+ qemuMonitorCloseFileHandle(mon, vhostfd_name, true) < 0)
VIR_WARN("failed to close device handle '%s'", vhostfd_name);
}
@@ -2258,7 +2274,7 @@ int qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon,
ret = qemuMonitorTextAddDevice(mon, devicestr);
if (ret < 0 && fd >= 0) {
- if (qemuMonitorCloseFileHandle(mon, fdname) < 0)
+ if (qemuMonitorCloseFileHandle(mon, fdname, true) < 0)
VIR_WARN("failed to close device handle '%s'", fdname);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 893f3e9..71ee932 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -363,7 +363,8 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon,
int fd);
int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
- const char *fdname);
+ const char *fdname,
+ bool preserveError);
/* XXX do we really want to hardcode 'netstr' as the
--
1.7.6
13 years, 5 months
