Re: [libvirt] [PATCH v2 3/3] qemu: Implement virDomainPMWakeup API
On 25.02.2012 01:31, Eric Blake wrote:
On 02/23/2012 01:44 AM, Michal Privoznik wrote:
On 15.02.2012 16:04, Michal Privoznik wrote:
using 'system-wakeup' monitor command. It is supported only in JSON,
as we are enabling it if possible. Moreover, this command is available
in qemu-1.1+ which definitely has JSON.
---
src/qemu/qemu_driver.c | 55
++
src/qemu/qemu_monitor.c | 19 ++
src/qemu/qemu_monitor.h |2 +
src/qemu/qemu_monitor_json.c | 21
src/qemu/qemu_monitor_json.h |2 +
5 files changed, 99 insertions(+), 0 deletions(-)
Ping? Eric, it seems to me like you've forgotten this last patch.
Indeed, it fell off my stack of most-recently-pinged patches. Reviewing
now, and thanks for the ping...
using 'system-wakeup' monitor command. It is supported only in JSON,
as we are enabling it if possible. Moreover, this command is available
in qemu-1.1+ which definitely has JSON.
---
src/qemu/qemu_driver.c | 55
++
src/qemu/qemu_monitor.c | 19 ++
src/qemu/qemu_monitor.h |2 +
src/qemu/qemu_monitor_json.c | 21
src/qemu/qemu_monitor_json.h |2 +
5 files changed, 99 insertions(+), 0 deletions(-)
+static int qemuDomainPMWakeup(virDomainPtr dom,
+ unsigned int flags)
Style nit - we aren't very consistent on whether function names begin on
line 1, but qemu_driver tends to use:
static int
qemuDomainPMWakeup(virDomainPtr dom,
unsigned int flags)
+++ b/src/qemu/qemu_monitor_json.c
@@ -3492,3 +3492,24 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr
mon,
virJSONValueFree(result);
return ret;
}
+
+int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon)
+{
+int ret = -1;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+
+cmd = qemuMonitorJSONMakeCommand(system_wakeup, NULL);
Seems so simple :)
ACK.
Thanks for review. Fixed all nits and pushed;
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0
On 24.02.2012 11:34, Christophe Fergeau wrote:
It's possible to disable SPICE TLS in qemu.conf. When this happens,
libvirt ignores any SPICE TLS port or x509 directory that may have
been set when it builds the qemu command line to use. However, it's
not ignoring the secure channels that may have been set and adds
tls-channel arguments to qemu command line.
Current qemu versions don't report an error when this happens, and try to use
TLS for the specified channels.
Before this patch
domain type='kvm'
nameauto-tls-port/name
memory65536/memory
os
type arch='x86_64' machine='pc'hvm/type
/os
devices
graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0'
ke
listen type='address' address='0'/
channel name='main' mode='secure'/
channel name='inputs' mode='secure'/
/graphics
/devices
/domain
generates
-spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs
and starts QEMU.
After this patch, an error is reported if a TLS port is set in the XML
or if secure channels are specified but TLS is disabled in qemu.conf.
This is the behaviour the oVirt people (where I spotted this issue) said
they would expect.
This fixes bug #790436
---
src/qemu/qemu_command.c | 12 +++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5a34504..4f3e61e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5231,7 +5231,12 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(opt, port=%u,
def-graphics[0]-data.spice.port);
-if (driver-spiceTLS def-graphics[0]-data.spice.tlsPort != -1)
+if (def-graphics[0]-data.spice.tlsPort != -1)
+if (!driver-spiceTLS) {
+qemuReportError(VIR_ERR_XML_ERROR,
+_(spice TLS port set in XML configuration,
but TLS is disabled in qemu.conf));
+goto error;
+}
virBufferAsprintf(opt, ,tls-port=%u,
def-graphics[0]-data.spice.tlsPort);
In fact, this needs to be wrapped with curly braces as the check for
tlsPort != -1 is meant to protect virBufferAsprintf() in the first
place. Sorry for not catching this earlier.
As an act of repentance I'll send patch.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't emit tls-port spice option if port is -1
On 28.02.2012 14:16, Jiri Denemark wrote: Bug introduced by commit eda0fc7a. --- src/qemu/qemu_command.c |9 ++--- 1 files changed, 6 insertions(+), 3 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: Fix libvirt name in qemu commandline namespace URL
s/libirt/libvirt/g --- Pushed under trivial rule. docs/drvqemu.html.in |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index fc76829..9afae13 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -551,7 +551,7 @@ $ virsh domxml-to-native qemu-argv demo.xml (span class=sinceSince 0.8.3/span). In order to use the XML additions, it is necessary to issue an XML namespace request (the special codexmlns:iname/i/code attribute) that - pulls in codehttp://libirt.org/schemas/domain/qemu/1.0/code; + pulls in codehttp://libvirt.org/schemas/domain/qemu/1.0/code; typically, the namespace is given the name of codeqemu/code. With the namespace in place, it is then possible to add an element codelt;qemu:commandlinegt;/code @@ -571,7 +571,7 @@ $ virsh domxml-to-native qemu-argv demo.xml /dl pExample:/ppre -lt;domain type='qemu' xmlns:qemu='http://libirt.org/schemas/domain/qemu/1.0'gt; +lt;domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'gt; lt;namegt;QEmu-fedora-i686lt;/namegt; lt;memorygt;219200lt;/memorygt; lt;osgt; -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: fix typo
* src/storage/storage_driver.c (storageVolumeWipeInternal): s/ pfitzner33/pfitzner33/. --- src/storage/storage_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 540e5d7..9130a40 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1937,7 +1937,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def, alg_char = pfitzner7; break; case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33: -alg_char = pfitzner33; +alg_char = pfitzner33; break; case VIR_STORAGE_VOL_WIPE_ALG_RANDOM: alg_char = random; -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: fix typo
On 29.02.2012 11:53, Daniel P. Berrange wrote: On Wed, Feb 29, 2012 at 11:44:47AM +0100, Michal Privoznik wrote: * src/storage/storage_driver.c (storageVolumeWipeInternal): s/ pfitzner33/pfitzner33/. --- src/storage/storage_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 540e5d7..9130a40 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1937,7 +1937,7 @@ storageVolumeWipeInternal(virStorageVolDefPtr def, alg_char = pfitzner7; break; case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33: -alg_char = pfitzner33; +alg_char = pfitzner33; break; case VIR_STORAGE_VOL_WIPE_ALG_RANDOM: alg_char = random; ACK Daniel Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu_conf: Move paths initialization to qemudLoadDriverConfig
This is needed as next patch allows their
modification via qemu.conf.
---
src/qemu/qemu_conf.c | 51 -
src/qemu/qemu_conf.h |4 +-
src/qemu/qemu_driver.c | 101 ++--
3 files changed, 82 insertions(+), 74 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e95c7a5..a22f3c7 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -67,7 +67,9 @@ void qemuDriverUnlock(struct qemud_driver *driver)
int qemudLoadDriverConfig(struct qemud_driver *driver,
- const char *filename) {
+ const char *filename,
+ const char *userdir,
+ const char *base) {
virConfPtr conf;
virConfValuePtr p;
char *user;
@@ -486,6 +488,53 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
CHECK_TYPE(keepalive_count, VIR_CONF_LONG);
if (p) driver-keepAliveCount = p-l;
+
+if (driver-privileged) {
+if (virAsprintf(driver-logDir,
+%s/log/libvirt/qemu, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+
+if (virAsprintf(driver-stateDir,
+ %s/run/libvirt/qemu, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+
+if (virAsprintf(driver-libDir,
+ %s/lib/libvirt/qemu, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+
+if (virAsprintf(driver-cacheDir,
+ %s/cache/libvirt/qemu, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-saveDir,
+ %s/lib/libvirt/qemu/save, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-snapshotDir,
+%s/lib/libvirt/qemu/snapshot, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-autoDumpPath,
+%s/lib/libvirt/qemu/dump, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+} else {
+if (virAsprintf(driver-logDir,
+%s/.libvirt/qemu/log, userdir) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-stateDir, %s/qemu/run, base) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-libDir, %s/qemu/lib, base) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-cacheDir, %s/qemu/cache, base) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-saveDir, %s/qemu/save, base) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-snapshotDir, %s/qemu/snapshot, base) == -1)
+goto out_of_memory;
+if (virAsprintf(driver-autoDumpPath, %s/qemu/dump, base) == -1)
+goto out_of_memory;
+}
virConfFree (conf);
return 0;
+
+out_of_memory:
+virReportOOMError();
+return -1;
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 36f1c4c..186404b 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -171,7 +171,9 @@ struct _qemuDomainCmdlineDef {
void qemuDriverLock(struct qemud_driver *driver);
void qemuDriverUnlock(struct qemud_driver *driver);
int qemudLoadDriverConfig(struct qemud_driver *driver,
- const char *filename);
+ const char *filename,
+ const char *userdir,
+ const char *base);
struct qemuDomainDiskInfo {
bool removable;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c6bdd29..7e11d15 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -420,6 +420,7 @@ cleanup:
static int
qemudStartup(int privileged) {
char *base = NULL;
+char *userdir = NULL;
char *driverConf = NULL;
int rc;
virConnectPtr conn = NULL;
@@ -451,70 +452,47 @@ qemudStartup(int privileged) {
virBitmapAlloc(QEMU_VNC_PORT_MAX - QEMU_VNC_PORT_MIN)) == NULL)
goto out_of_memory;
-/* read the host sysinfo */
-if (privileged)
-qemu_driver-hostsysinfo = virSysinfoRead();
-
+/* Configuration paths are either ~/.libvirt/qemu/... (session) or
+ * /etc/libvirt/qemu/... (system).
+ */
if (privileged) {
-if (virAsprintf(qemu_driver-logDir,
-%s/log/libvirt/qemu, LOCALSTATEDIR) == -1)
-goto out_of_memory;
-
if ((base = strdup (SYSCONFDIR /libvirt)) == NULL)
goto out_of_memory;
-
-if (virAsprintf(qemu_driver-stateDir,
- %s/run/libvirt/qemu, LOCALSTATEDIR) == -1)
-goto out_of_memory;
-
-if (virAsprintf(qemu_driver-libDir,
- %s/lib/libvirt/qemu, LOCALSTATEDIR) == -1)
-goto out_of_memory;
-
-if (virAsprintf(qemu_driver-cacheDir,
- %s/cache/libvirt/qemu, LOCALSTATEDIR) == -1)
-goto out_of_memory;
-if
[libvirt] [PATCH 2/2] qemu_conf: Allow overriding of path for managed-save
This patch allows users to set different path for managed-save
images than under /var/lib/libvirt/qemu/save; Some sysadmins
do have small / and thus /var but don't want to compile libvirt
on their own just to override this path.
---
src/qemu/qemu.conf |7 +++
src/qemu/qemu_conf.c | 15 ---
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 95428c1..ae634ef 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -350,3 +350,10 @@
#
#keepalive_interval = 5
#keepalive_count = 5
+
+###
+# Configurable paths:
+# This allows user to override some paths libvirt uses for storing
+# files like managed-save, etc.
+#
+# managed_save_dir = /var/lib/libvirt/qemu/save/
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a22f3c7..0b06ef2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -505,9 +505,18 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
if (virAsprintf(driver-cacheDir,
%s/cache/libvirt/qemu, LOCALSTATEDIR) == -1)
goto out_of_memory;
-if (virAsprintf(driver-saveDir,
- %s/lib/libvirt/qemu/save, LOCALSTATEDIR) == -1)
-goto out_of_memory;
+
+p = virConfGetValue (conf, managed_save_dir);
+CHECK_TYPE (managed_save_dir, VIR_CONF_STRING);
+if (p p-str) {
+if (!(driver-saveDir = strdup(p-str)))
+goto out_of_memory;
+} else {
+if (virAsprintf(driver-saveDir,
+%s/lib/libvirt/qemu/save, LOCALSTATEDIR) == -1)
+goto out_of_memory;
+}
+
if (virAsprintf(driver-snapshotDir,
%s/lib/libvirt/qemu/snapshot, LOCALSTATEDIR) == -1)
goto out_of_memory;
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Make managed-save path configurable
Some users/sysadmins don't really create big partition for / or /var; Therefore number of domains that can be managed-save is limited to only a few because we simply run out of free space. Yes, one can override the path at compile time, but it's sort of overkill. Having said that, I think this is nice to have feature. We can make more paths configurable if this gains enough fame. Michal Privoznik (2): qemu_conf: Move paths initialization to qemudLoadDriverConfig qemu_conf: Allow overriding of path for managed-save src/qemu/qemu.conf |7 +++ src/qemu/qemu_conf.c | 60 - src/qemu/qemu_conf.h |4 +- src/qemu/qemu_driver.c | 101 ++-- 4 files changed, 98 insertions(+), 74 deletions(-) -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
Some nits are generated during XML parse (e.g. MAC address of
an interface); However, with current implementation, if we
are plugging a device both to persistent and live config,
we parse given XML twice: first time for live, second for config.
This is wrong then as the second time we are not guaranteed
to generate same values as we did for the first time.
To prevent that we need to create a copy of DeviceDefPtr;
This is done through format/parse process instead of writing
functions for deep copy as it is easier to maintain:
adding new field to any virDomain*DefPtr doesn't require change
of copying function.
---
src/conf/domain_conf.c | 94 ++
src/conf/domain_conf.h |3 +
src/libvirt_private.syms |1 +
src/qemu/qemu_driver.c | 40 +++
4 files changed, 121 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9654f1..f001319 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14118,3 +14118,97 @@ virDomainNetFind(virDomainDefPtr def, const char
*device)
return net;
}
+
+#define VIR_DOMAIN_DEVICE_FORMAT(func) \
+if (func 0) \
+goto cleanup; \
+xmlStr = virBufferContentAndReset(buf); \
+ret = virDomainDeviceDefParse(caps, def, xmlStr, flags)
+
+virDomainDeviceDefPtr
+virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src)
+{
+virDomainDeviceDefPtr ret = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int flags = VIR_DOMAIN_XML_INACTIVE;
+char *xmlStr = NULL;
+
+switch(src-type) {
+case VIR_DOMAIN_DEVICE_DISK:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainDiskDefFormat(buf,
+src-data.disk,
+flags));
+break;
+case VIR_DOMAIN_DEVICE_LEASE:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainLeaseDefFormat(buf,
+ src-data.lease));
+break;
+case VIR_DOMAIN_DEVICE_FS:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainFSDefFormat(buf,
+ src-data.fs,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_NET:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainNetDefFormat(buf,
+ src-data.net,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_INPUT:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainInputDefFormat(buf,
+ src-data.input,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_SOUND:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainSoundDefFormat(buf,
+ src-data.sound,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_VIDEO:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainVideoDefFormat(buf,
+ src-data.video,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainHostdevDefFormat(buf,
+ src-data.hostdev,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_WATCHDOG:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainWatchdogDefFormat(buf,
+src-data.watchdog,
+flags));
+break;
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainControllerDefFormat(buf,
+
src-data.controller,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainGraphicsDefFormat(buf,
+src-data.graphics,
+flags));
+break;
+case VIR_DOMAIN_DEVICE_HUB:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainHubDefFormat(buf,
+ src-data.hub,
+ flags));
+break;
+case VIR_DOMAIN_DEVICE_REDIRDEV:
+VIR_DOMAIN_DEVICE_FORMAT(virDomainRedirdevDefFormat(buf,
+src-data.redirdev,
+flags));
+break;
+default:
+
Re: [libvirt] [PATCH] qemu: Shared or readonly disks are always safe wrt migration
On 05.03.2012 15:15, Jiri Denemark wrote: No matter what cache mode is used, readonly disks are always safe wrt migration. Shared disks are required to be readonly or to disable host-side cache, which makes them safe as well. --- src/qemu/qemu_migration.c |8 +--- 1 files changed, 5 insertions(+), 3 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: eliminate crash in virNetDevMacVLanCreateWithVPortProfile
On 05.03.2012 20:42, Laine Stump wrote: From: root r...@vlap.laine.org Commit 723d5c (added after the release of 0.9.10) adds a NetlinkEventClient for each interface sent to virNetDevMacVLanCreateWithVPortProfile. This should only be done if the interface actually *has* a virtPortProfile, otherwise the event handler would be a NOP. The bigger problem is that part of the setup to create the NetlinkEventClient is to do a memcpy of virtPortProfile - if it's NULL, this triggers a segv. This patch just qualifies the code that adds the client - if virtPortProfile is NULL, it's skipped. --- src/util/virnetdevmacvlan.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/17] qemu: refactor hotplug detach of hostdevs
On 28.02.2012 21:14, Laine Stump wrote: This refactoring is necessary to support hotplug detach of type=hostdev network devices, but needs to be in a separate patch to make potential debugging of regressions more practical. Rather than the lowest level functions searching for a matching device, the search is now done in the toplevel function, and an intermediate-level function (qemuDomainDetachThisHostDevice()), which expects that the device's entry is already found, is called (this intermediate function will be called by qemuDomainDetachNetDevice() in order to support detach of type=hostdev net devices) This patch should result in 0 differences in functionality. --- New patch for V2. src/qemu/qemu_hotplug.c | 228 +++ 1 files changed, 93 insertions(+), 135 deletions(-) Nice cleanup. I wish we have more like these :) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/17] qemu: support type='hostdev' network devices at domain start
On 28.02.2012 21:14, Laine Stump wrote: This patch makes sure that each network device (interface) of type='hostdev' appears on both the hostdevs list and the nets list of the virDomainDef, and it modifies the qemu driver startup code so that these devices will be presented to qemu on the commandline as hostdevs rather than as network devices. It does not add support for hotplug of these type of devices, or code to honor the mac address or virtualport given in the config (both of those will be done in separate patches). Once each device is placed on both lists, much of what this patch does is modify places in the code that traverse all the device lists so that these hybrid devices are only acted on once - either along with the other hostdevs, or along with the other interfaces. (In many cases, only one of the lists is traversed / a specific operation is performed on only one type of device. In those instances, the code can remain unchanged.) There is one special case - when building the commandline, interfaces are allowed to proceed all the way through networkAllocateActualDevice() before deciding to skip - this is so that (once we have support for networks with pools of hostdev devices) we can get the actual device allocated, then rely on the loop processing all hostdevs to generate the correct commandline. --- New patch in V2. src/conf/domain_conf.c | 54 +--- src/qemu/qemu_command.c| 36 -- .../qemuxml2argvdata/qemuxml2argv-net-hostdev.args |7 +++ .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml |7 --- tests/qemuxml2argvtest.c |2 + 5 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.args ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/17] conf: change virDomainNetRemove from static to global
On 28.02.2012 21:14, Laine Stump wrote:
This exact code is duplicated in qemuDomainDetachNetDevice().
---
New patch in V2.
(yeah, I just noticed the movement of the virDomainHostdevXX()
declarations in this patch; I guess I was rearranging for consistent
ordering. If this concerns anyone, I can squash it out before I push.)
src/conf/domain_conf.c |2 +-
src/conf/domain_conf.h |5 +++--
src/libvirt_private.syms |1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7135024..b994718 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7084,7 +7084,7 @@ int virDomainNetIndexByMac(virDomainDefPtr def, const
unsigned char *mac)
return -1;
}
-static void virDomainNetRemove(virDomainDefPtr def, size_t i)
+void virDomainNetRemove(virDomainDefPtr def, size_t i)
{
virDomainNetDefPtr net = def-nets[i];
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6ab5f32..a9426b3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1902,12 +1902,13 @@ int virDomainDiskRemoveByName(virDomainDefPtr def,
const char *name);
int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac);
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
+void virDomainNetRemove(virDomainDefPtr def, size_t i);
int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac);
-int virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr
hostdev);
-void virDomainHostdevRemove(virDomainDefPtr def, size_t i);
int virDomainHostdevFind(virDomainDefPtr def, virDomainHostdevDefPtr match,
virDomainHostdevDefPtr *found);
+int virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr
hostdev);
+void virDomainHostdevRemove(virDomainDefPtr def, size_t i);
Maybe my brain is too small for this, but this looks useless to me.
However, not a show stopper.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/17] conf: parse/format type='hostdev' network interfaces
On 28.02.2012 21:14, Laine Stump wrote:
This is the new interface type that sets up a PCI/USB network device
to be assigned to the guest with PCI/USB passthrough after
initializing some network device-specific things from the config
(e.g. MAC address, virtualport profile parameters). Here is an example
of the syntax:
interface type='hostdev' managed='yes'
source
address type='pci' domain='0' bus='0' slot='4' function='0'/
/source
mac address='00:11:22:33:44:55'/
address type='pci' domain='0' bus='0' slot='7' function='0'/
/interface
This would assign the PCI card from bus 0 slot 4 function 0 on the
host, to bus 0 slot 7 function 0 on the guest, but would first set the
MAC address of the card to 00:11:22:33:44:55.
Although it's not expected to be used very much, usb network hostdevs
are also supported for completeness. source syntax is identical to
that for plain hostdev devices, except that the address element
should have type='usb' added if it's specified:
interface type='hostdev'
source
address type='usb' bus='0' device='4'/
/source
mac address='00:11:22:33:44:55'/
/interface
If the vendor/product form of usb specification is used, type='usb'
is implied:
interface type='hostdev'
source
vendor id='0x0012'/
product id='0x24dd'/
/source
mac address='00:11:22:33:44:55'/
/interface
---
V2: address Eric's concerns from V1
- check for OOM after strdup
- put in a NOP virDomainHostdevDefClear() rather than just commenting
there is nothing in the HostdevDef that needs to be freed
- eliminate inconsistent {} usage.
docs/formatdomain.html.in | 41 +
docs/schemas/domaincommon.rng | 50 +++
src/conf/domain_conf.c | 154
++--
src/conf/domain_conf.h | 10 ++
src/libvirt_private.syms |1 +
src/qemu/qemu_command.c|1 +
src/uml/uml_conf.c |5 +
src/xenxs/xen_sxpr.c |1 +
.../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 48 ++
tests/qemuxml2xmltest.c|1 +
10 files changed, 297 insertions(+), 15 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
new file mode 100644
index 000..504e4f6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
@@ -0,0 +1,48 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory219136/memory
+ currentMemory219136/currentMemory
+ vcpu1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+ source dev='/dev/HostVG/QEMUGuest1'/
+ target dev='hda' bus='ide'/
+ address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+interface type='hostdev' managed='yes'
+ mac address='00:11:22:33:44:55'/
+ source
+address type='pci' domain='0x0002' bus='0x03' slot='0x07'
function='0x1'/
+ /source
+ virtualport type='802.1Qbg'
+parameters managerid='11' typeid='1193047' typeidversion='2'
instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/
+ /virtualport
+/interface
+interface type='hostdev'
+ mac address='11:11:22:33:44:55'/
+ source
+address type='usb' bus='0' device='2'/
+ /source
+/interface
+interface type='hostdev'
+ mac address='22:11:22:33:44:55'/
+ source
+vendor id='0x0012'/
+product id='0x24dd'/
+ /source
+/interface
This looks odd to me; I mean why add this interface here but remove it
in the very next patch? Maybe to prove/test that parsing and formatting
works correctly even for this case.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/17] qemu: use virDomainNetRemove instead of inline code
On 28.02.2012 21:14, Laine Stump wrote: The code being replaced is exactly identical to the newly global function, right down to the comment. --- New patch in V2 src/qemu/qemu_hotplug.c | 14 +- 1 files changed, 1 insertions(+), 13 deletions(-) ACK definitely. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach
On 28.02.2012 21:14, Laine Stump wrote:
qemuDomainAttachNetDevice
- re-ordered some things at start of function because
networkAllocateActualDevice should always be run and a slot
in def-nets always allocated, but host_net_add isn't needed
if the actual type is hostdev.
- if actual type is hostdev, defer to
qemuDomainAttachHostDevice (which will reach up to the NetDef
for things like MAC address when necessary). After return
from qemuDomainAttachHostDevice, slip directly to cleanup,
since the rest of the function is specific to emulated net
devices.
- put assignment of new NetDef into expanded def-nets down
below cleanup: (but only on success) since it is also needed
for emulated and hostdev net devices.
qemuDomainDetachHostDevice
- after locating the exact device to detach, check if it's a
network device and, if so, use toplevel
qemuDomainDetachNetDevice instead so that the def-nets list
is properly updated, and 'actual device' properly returned to
network pool if appropriate. Otherwise, for normal hostdevs,
call the lower level qemuDomainDetachThisDevice.
qemuDomainDetachNetDevice
- This is where it gets a bit tricky. After locating the device
on the def-nets list, if the network device type == hostdev,
call the *lower level* qemuDomainDetachThisDevice (which will
reach back up to the parent net device for MAC address /
virtualport when appropriate, then clear the device out of
def-hostdevs) before skipping past all the emulated
net-device-specific code to cleanup:, where the network
device is removed from def-nets, and the network device
object is freed.
In short, any time a hostdev-type network device is detached, we must
go through the toplevel virDomaineDetachNetDevice function first and
last, to make sure 1) the def-nnets list is properly managed, and 2)
any device allocated with networkAllocateActualDevice is properly
freed. At the same time, in the middle we need to go through the
lower-level virDomainDetach*This*HostDevice to be sure that 1) the
def-hostdevs list is properly managed, 2) the PCI device is properly
detached from the guest and reattached to the host (if appropriate),
and 3) any higher level setup/teardown is called at the appropriate
time, by reaching back up to the NetDef config (part (3) will be
covered in a separate patch).
---
src/qemu/qemu_hotplug.c | 61 +-
1 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6119108..50563c5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -661,9 +661,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
bool iface_connected = false;
int actualType;
-if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
-qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-_(installed qemu version does not support
host_net_add));
+/* preallocate new slot for device */
+if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets+1) 0) {
+virReportOOMError();
return -1;
}
@@ -672,9 +672,27 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
* to the one defined in the network definition.
*/
if (networkAllocateActualDevice(net) 0)
-goto cleanup;
+return -1;
Okay, vm-def-nets won't leak, but will be one item bigger; Do we want
to realloc it back as we do in all detach functions, e.g.
virDomainNetRemove() ? The vm-def-nnets counter isn't changed yet so I
guess this is alright.
Anyway, looking good so ACK.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
Some nits are generated during XML parse (e.g. MAC address of
an interface); However, with current implementation, if we
are plugging a device both to persistent and live config,
we parse given XML twice: first time for live, second for config.
This is wrong then as the second time we are not guaranteed
to generate same values as we did for the first time.
To prevent that we need to create a copy of DeviceDefPtr;
This is done through format/parse process instead of writing
functions for deep copy as it is easier to maintain:
adding new field to any virDomain*DefPtr doesn't require change
of copying function.
---
diff to v1:
-Eric's review included
src/conf/domain_conf.c | 70 ++
src/conf/domain_conf.h |3 ++
src/libvirt_private.syms |1 +
src/qemu/qemu_driver.c | 38 ++---
4 files changed, 95 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b994718..42cfbd5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14529,3 +14529,73 @@ virDomainNetFind(virDomainDefPtr def, const char
*device)
return net;
}
+
+virDomainDeviceDefPtr
+virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src)
+{
+virDomainDeviceDefPtr ret = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int flags = VIR_DOMAIN_XML_INACTIVE;
+char *xmlStr = NULL;
+int rc = -1;
+
+switch(src-type) {
+case VIR_DOMAIN_DEVICE_DISK:
+rc = virDomainDiskDefFormat(buf, src-data.disk, flags);
+break;
+case VIR_DOMAIN_DEVICE_LEASE:
+rc = virDomainLeaseDefFormat(buf, src-data.lease);
+break;
+case VIR_DOMAIN_DEVICE_FS:
+rc = virDomainFSDefFormat(buf, src-data.fs, flags);
+break;
+case VIR_DOMAIN_DEVICE_NET:
+rc = virDomainNetDefFormat(buf, src-data.net, flags);
+break;
+case VIR_DOMAIN_DEVICE_INPUT:
+rc = virDomainInputDefFormat(buf, src-data.input, flags);
+break;
+case VIR_DOMAIN_DEVICE_SOUND:
+rc = virDomainSoundDefFormat(buf, src-data.sound, flags);
+break;
+case VIR_DOMAIN_DEVICE_VIDEO:
+rc = virDomainVideoDefFormat(buf, src-data.video, flags);
+break;
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+rc = virDomainHostdevDefFormat(buf, src-data.hostdev, flags);
+break;
+case VIR_DOMAIN_DEVICE_WATCHDOG:
+rc = virDomainWatchdogDefFormat(buf, src-data.watchdog, flags);
+break;
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+rc = virDomainControllerDefFormat(buf, src-data.controller, flags);
+break;
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+rc = virDomainGraphicsDefFormat(buf, src-data.graphics, flags);
+break;
+case VIR_DOMAIN_DEVICE_HUB:
+rc = virDomainHubDefFormat(buf, src-data.hub, flags);
+break;
+case VIR_DOMAIN_DEVICE_REDIRDEV:
+rc = virDomainRedirdevDefFormat(buf, src-data.redirdev, flags);
+break;
+default:
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Copying definition of '%d' type
+ is not implemented yet.),
+ src-type);
+goto cleanup;
+}
+
+if (rc 0)
+goto cleanup;
+
+xmlStr = virBufferContentAndReset(buf);
+ret = virDomainDeviceDefParse(caps, def, xmlStr, flags);
+
+cleanup:
+VIR_FREE(xmlStr);
+return ret;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 87b4103..1453822 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1794,6 +1794,9 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainHubDefFree(virDomainHubDefPtr def);
void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def);
void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
+virDomainDeviceDefPtr virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src);
int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
int type);
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c44c617..a4cf199 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -287,6 +287,7 @@ virDomainDeviceAddressIsValid;
virDomainDeviceAddressPciMultiTypeFromString;
virDomainDeviceAddressPciMultiTypeToString;
virDomainDeviceAddressTypeToString;
+virDomainDeviceDefCopy;
virDomainDeviceDefFree;
virDomainDeviceDefParse;
virDomainDeviceInfoIterate;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ddb381a..1d121cf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5563,7 +5563,7 @@
[libvirt] [PATCH] qemu: Fix startupPolicy for snapshot-revert
Currently, startupPolicy='requisite' was determining cold boot
by migrateForm != NULL. That means, if domain was started up
with migrateForm set we didn't require disk source path and allowed
it to be dropped. However, on snapshot-revert domain wasn't migrated
but according to documentation, requisite should drop disk source
as well.
---
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=798938
src/qemu/qemu_driver.c| 16 +---
src/qemu/qemu_migration.c |2 +-
src/qemu/qemu_process.c |3 ++-
src/qemu/qemu_process.h |1 +
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ddb381a..b17b52c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1358,7 +1358,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn,
const char *xml,
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup; /* free the 'vm' we created ? */
-if (qemuProcessStart(conn, driver, vm, NULL,
+if (qemuProcessStart(conn, driver, vm, NULL, true,
(flags VIR_DOMAIN_START_PAUSED) != 0,
(flags VIR_DOMAIN_START_AUTODESTROY) != 0,
-1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE)
0) {
@@ -4107,8 +4107,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
}
/* Set the migration source and start it up. */
-ret = qemuProcessStart(conn, driver, vm, stdio, true,
- false, *fd, path, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
+ret = qemuProcessStart(conn, driver, vm, stdio, false, true,
+ false, *fd, path, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
if (intermediatefd != -1) {
if (ret 0) {
@@ -4709,8 +4710,9 @@ qemuDomainObjStart(virConnectPtr conn,
}
}
-ret = qemuProcessStart(conn, driver, vm, NULL, start_paused,
- autodestroy, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused,
+ autodestroy, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
virDomainAuditStart(vm, booted, ret = 0);
if (ret = 0) {
virDomainEventPtr event =
@@ -10788,7 +10790,7 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virDomainObjAssignDef(vm, config, false);
rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- true, false, -1, NULL, snap,
+ false, true, false, -1, NULL, snap,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
virDomainAuditStart(vm, from-snapshot, rc = 0);
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
@@ -10878,7 +10880,7 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (event)
qemuDomainEventQueue(driver, event);
rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- paused, false, -1, NULL, NULL,
+ false, paused, false, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
virDomainAuditStart(vm, from-snapshot, rc = 0);
if (rc 0) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 77d40c0..92d046a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1229,7 +1229,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
/* Start the QEMU daemon, with the same command-line arguments plus
* -incoming $migrateFrom
*/
-if (qemuProcessStart(dconn, driver, vm, migrateFrom, true,
+if (qemuProcessStart(dconn, driver, vm, migrateFrom, false, true,
true, dataFD[0], NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) 0) {
virDomainAuditStart(vm, migrated, false);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7b99814..502de89 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3072,6 +3072,7 @@ int qemuProcessStart(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
const char *migrateFrom,
+ bool cold_boot,
bool start_paused,
bool autodestroy,
int stdin_fd,
@@ -3227,7 +3228,7 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
VIR_DEBUG(Checking for CDROM and floppy presence);
-if (qemuDomainCheckDiskPresence(driver, vm, migrateFrom != NULL) 0)
+if (qemuDomainCheckDiskPresence(driver, vm, cold_boot) 0)
goto cleanup;
Re: [libvirt] [PATCH] qemu: Fix startupPolicy for snapshot-revert
On 07.03.2012 19:36, Eric Blake wrote: On 03/07/2012 11:19 AM, Michal Privoznik wrote: Currently, startupPolicy='requisite' was determining cold boot by migrateForm != NULL. That means, if domain was started up with migrateForm set we didn't require disk source path and allowed s/migrateForm/migrateFrom/ (twice) it to be dropped. However, on snapshot-revert domain wasn't migrated but according to documentation, requisite should drop disk source as well. --- Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=798938 src/qemu/qemu_driver.c| 16 +--- src/qemu/qemu_migration.c |2 +- src/qemu/qemu_process.c |3 ++- src/qemu/qemu_process.h |1 + 4 files changed, 13 insertions(+), 9 deletions(-) @@ -4107,8 +4107,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ -ret = qemuProcessStart(conn, driver, vm, stdio, true, - false, *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); +ret = qemuProcessStart(conn, driver, vm, stdio, false, true, + false, *fd, path, NULL, Yuck - we're starting to rack up so many bools that it's hard to tell which one is which. This patch can go in as-is, but I'd also like to see a followup patch that refactors things into using an 'unsigned int flags' with an internal enum for bit values (QEMU_START_COLD, QEMU_START_PAUSED, QEMU_START_AUTODESTROY, ...). ACK. Thanks pushed. And I'll send patch for what you've described. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
On 07.03.2012 19:46, Laine Stump wrote: On 03/07/2012 12:48 PM, Michal Privoznik wrote: Some nits are generated during XML parse (e.g. MAC address of an interface); However, with current implementation, if we are plugging a device both to persistent and live config, we parse given XML twice: first time for live, second for config. This is wrong then as the second time we are not guaranteed to generate same values as we did for the first time. To prevent that we need to create a copy of DeviceDefPtr; This is done through format/parse process instead of writing functions for deep copy as it is easier to maintain: adding new field to any virDomain*DefPtr doesn't require change of copying function. --- diff to v1: -Eric's review included src/conf/domain_conf.c | 70 ++ src/conf/domain_conf.h |3 ++ src/libvirt_private.syms |1 + src/qemu/qemu_driver.c | 38 ++--- 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b994718..42cfbd5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14529,3 +14529,73 @@ virDomainNetFind(virDomainDefPtr def, const char *device) return net; } I still think there should be a comment added here saying something like: NB: virDomainDeviceDefCopy does a deep copy of only the parts of a DeviceDef that are valid when just the flag VIR_DOMAIN_XML_INACTIVE is set. This means that any part of the device xml that is conditionally parsed/formatted based on some other flag being set (or on the INACTIVE flag being reset) *will not* be copied to the destination. Caveat emptor. + +virDomainDeviceDefPtr +virDomainDeviceDefCopy(virCapsPtr caps, + const virDomainDefPtr def, + virDomainDeviceDefPtr src) Otherwise it looks like you've taken care of all of Eric's issues, and seems clean, so ACK from me (conditional on adding a comment documenting the limitations of the new copy function). Thank you both; Added comment and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: Don't overflow on errno in virFileAccessibleAs
If we need to virFork() to check assess() under different
UID+GID we need to translate returned status via WEXITSTATUS().
Otherwise, we may return values greater than 255 which is
obviously wrong.
---
src/util/util.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 548ed1c..15e6cfa 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -724,8 +724,13 @@ virFileAccessibleAs(const char *path, int mode,
return -1;
}
+if (!WIFEXITED(status)) {
+errno = EINTR;
+return -1;
+}
+
if (status) {
-errno = status;
+errno = WEXITSTATUS(status);
return -1;
}
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuProcessStart: Switch to flags instead of bunch booleans
Currently, we have 3 boolean arguments we have to pass
to qemuProcessStart(). As libvirt grows it is harder and harder
to remember them and their position. Therefore we should
switch to flags instead.
---
src/qemu/qemu_driver.c| 45 +
src/qemu/qemu_migration.c |7 ---
src/qemu/qemu_process.c | 21 +
src/qemu/qemu_process.h | 12
4 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2c4544..1aa3a28 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1326,10 +1326,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
virDomainPtr dom = NULL;
virDomainEventPtr event = NULL;
virDomainEventPtr event2 = NULL;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
virCheckFlags(VIR_DOMAIN_START_PAUSED |
VIR_DOMAIN_START_AUTODESTROY, NULL);
+if (flags VIR_DOMAIN_START_PAUSED)
+start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
+if (flags VIR_DOMAIN_START_AUTODESTROY)
+start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY;
+
qemuDriverLock(driver);
if (!(def = virDomainDefParseString(driver-caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
@@ -1358,10 +1364,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup; /* free the 'vm' we created ? */
-if (qemuProcessStart(conn, driver, vm, NULL, true,
- (flags VIR_DOMAIN_START_PAUSED) != 0,
- (flags VIR_DOMAIN_START_AUTODESTROY) != 0,
- -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE)
0) {
+if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ start_flags) 0) {
virDomainAuditStart(vm, booted, false);
if (qemuDomainObjEndJob(driver, vm) 0)
qemuDomainRemoveInactive(driver, vm);
@@ -4109,9 +4114,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
}
/* Set the migration source and start it up. */
-ret = qemuProcessStart(conn, driver, vm, stdio, false, true,
- false, *fd, path, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
+ret = qemuProcessStart(conn, driver, vm, stdio, *fd, path, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
+ VIR_QEMU_PROCESS_START_PAUSED);
if (intermediatefd != -1) {
if (ret 0) {
@@ -4681,6 +4686,10 @@ qemuDomainObjStart(virConnectPtr conn,
bool autodestroy = (flags VIR_DOMAIN_START_AUTODESTROY) != 0;
bool bypass_cache = (flags VIR_DOMAIN_START_BYPASS_CACHE) != 0;
bool force_boot = (flags VIR_DOMAIN_START_FORCE_BOOT) != 0;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
+
+start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
+start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0;
/*
* If there is a managed saved state restore it instead of starting
@@ -4712,9 +4721,8 @@ qemuDomainObjStart(virConnectPtr conn,
}
}
-ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused,
- autodestroy, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags);
virDomainAuditStart(vm, booted, ret = 0);
if (ret = 0) {
virDomainEventPtr event =
@@ -10795,9 +10803,10 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (config)
virDomainObjAssignDef(vm, config, false);
-rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- false, true, false, -1, NULL, snap,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+rc = qemuProcessStart(snapshot-domain-conn,
+ driver, vm, NULL, -1, NULL, snap,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED);
virDomainAuditStart(vm, from-snapshot, rc = 0);
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
event = virDomainEventNewFromObj(vm,
@@ -10882,12 +10891,16 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
/* Flush first event, now do transition 2 or 3 */
bool paused = (flags VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
+unsigned int
[libvirt] [PATCH] qemuBuildCommandLine: Don't add tlsPort if none set
If user hasn't supplied any tlsPort we default to setting it
to zero in our internal structure. However, when building command
line we test it against -1 which is obviously wrong.
---
src/qemu/qemu_command.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index de2d4a1..ed82cc2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(opt, port=%u, def-graphics[0]-data.spice.port);
-if (def-graphics[0]-data.spice.tlsPort != -1) {
+if (def-graphics[0]-data.spice.tlsPort) {
if (!driver-spiceTLS) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(spice TLS port set in XML configuration,
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Don't overflow on errno in virFileAccessibleAs
On 08.03.2012 14:08, Eric Blake wrote:
On 03/08/2012 03:37 AM, Michal Privoznik wrote:
If we need to virFork() to check assess() under different
UID+GID we need to translate returned status via WEXITSTATUS().
Otherwise, we may return values greater than 255 which is
obviously wrong.
---
src/util/util.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 548ed1c..15e6cfa 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -724,8 +724,13 @@ virFileAccessibleAs(const char *path, int mode,
return -1;
}
+if (!WIFEXITED(status)) {
+errno = EINTR;
+return -1;
+}
ACK; this matches what we do in virFileOpenForked.
Thanks pushed.
However, I still see two lingering issues that might be worth revisiting:
1. I wonder if virWaitPid() would be easier to use if it only returned
success on WIFEXITED, and set *status to WEXITSTAUS(), while returning
-1 on any child dying due to a signal. I'd have to audit the users of
virWaitPid to see if they can all be simplified by this change, or if
there really is a user that needs to know if a child exited due to a signal.
yes, i was wondering about this too when writing the patch. However I
took the quicker way. Let me see if i can produce cleanup patch as
you've described it.
2. This still shares the latent bug in virFileOpenForked that errno is
not always guaranteed to be less than 255; on GNU Hurd, this code is
broken - but libvirt doesn't compile on Hurd. A true fix would be to
enumerate specific errno values to specific exit codes, and map all
others to a catch-all; see how daemon/libvirtd.c has virDaemonErr for
this purpose.
Yeah, since we don't compile on Hurd anyway, I wouldn't take much care
here. I am not saying we should make it intentionally harder for a
developer trying to make libvirt compile there, but why unnecessarily
bound ourselves?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuBuildCommandLine: Don't add tlsPort if none set
On 08.03.2012 23:26, Eric Blake wrote:
On 03/08/2012 06:30 AM, Michal Privoznik wrote:
If user hasn't supplied any tlsPort we default to setting it
to zero in our internal structure. However, when building command
line we test it against -1 which is obviously wrong.
---
src/qemu/qemu_command.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index de2d4a1..ed82cc2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(opt, port=%u,
def-graphics[0]-data.spice.port);
-if (def-graphics[0]-data.spice.tlsPort != -1) {
+if (def-graphics[0]-data.spice.tlsPort) {
ACK.
Thanks, pushed. However, turned out I've needed to update one test as well:
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
index 62ce46f..ebda714 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args
@@ -10,6 +10,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test
LOGNAME=test QEMU_AUDIO_DRV=spice \
-device rtl8139,vlan=0,id=net0,mac=52:54:00:71:70:89,bus=pci.0,addr=0x7 \
-net tap,script=/etc/qemu-ifup,vlan=0,name=hostnet0 -serial pty \
-usb -device usb-tablet,id=input0 \
--spice port=5900,tls-port=0,x509-dir=/etc/pki/libvirt-spice -vga std \
+-spice port=5900,x509-dir=/etc/pki/libvirt-spice -vga std \
-device AC97,id=sound0,bus=pci.0,addr=0x3 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
That is, don't add wildcard tls-port zero to generated commandline.
Sorry for this incompleteness.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuxml2argvtest: Pass some additional flags to graphics-spice-agentmouse
One of the recent commits introduced support for spice agent-mouse. However, test for this feature require some tweaking: pass QEMU_CAPS_CHARDEV_SPICEVMC | QEMU_CAPS_NODEFCONFIG and add -vga cirrus. --- Since we default to '-vga cirrus' when QEMU_CAPS_VGA set, I've added it to XML as well for completeness. The other option would be to not set the capability. Pushed under build-breaker rule as qemuxml2argvtest fails without this. .../qemuxml2argv-graphics-spice-agentmouse.args|2 +- .../qemuxml2argv-graphics-spice-agentmouse.xml |3 +++ tests/qemuxml2argvtest.c |4 +++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args index 2c3ef06..4a7bd2e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.args @@ -5,4 +5,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ -hda /dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent \ -device virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 \ -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,agent-mouse=off,x509-dir=/etc/pki/libvirt-spice,tls-channel=main \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 +-vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.xml index facc7ac..95954fc 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.xml @@ -31,6 +31,9 @@ target type='virtio' name='com.redhat.spice.0'/ address type='virtio-serial' controller='1' bus='0' port='3'/ /channel +video + model type='cirrus' vram='9216' heads='1'/ +/video memballoon model='virtio'/ /devices /domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3cfd69c..9001834 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -513,7 +513,9 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE); DO_TEST(graphics-spice-agentmouse, false, QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, -QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE); +QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE, +QEMU_CAPS_CHARDEV_SPICEVMC, +QEMU_CAPS_NODEFCONFIG); DO_TEST(graphics-spice-compression, false, QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE); -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: eliminate redundant VIR_ALLOC of 1st element of network DNS hosts.
On 09.03.2012 10:03, Laine Stump wrote: virNetworkDNSHostsDefParseXML was calling VIR_ALLOC(def-hosts) if def-hosts was NULL. This is a waste of time, though, since VIR_REALLOC_N is called a few lines further down, prior to any use of def-hosts. (initializing def-nhosts to 0 is also redundant, because the newly allocated memory will always be cleared to all 0's anyway). --- src/conf/network_conf.c |8 1 files changed, 0 insertions(+), 8 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 8/8] add qemu cache mutex
On 11.03.2012 19:56, Lee Schermerhorn wrote:
Add a mutex for access to the qemu emulator cache. Not clear that
this is actually needed -- driver should be locked across calls [?].
The patch can be dropped if not needed.
---
src/qemu/qemu_capabilities.c | 18 +-
src/qemu/qemu_capabilities.h |2 ++
src/qemu/qemu_driver.c |3 +++
3 files changed, 22 insertions(+), 1 deletion(-)
Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c
===
--- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c
+++ libvirt-0.9.10/src/qemu/qemu_capabilities.c
@@ -27,6 +27,7 @@
#include memory.h
#include logging.h
#include virterror_internal.h
+#include threads.h
#include util.h
#include virfile.h
#include nodeinfo.h
@@ -180,6 +181,11 @@ enum qemuCapsProbes {
QEMU_PROBE_SIZE
};
+/*
+ * Use static initializer for tests
+ */
+static virMutex qemuEmulatorCacheMutex = { PTHREAD_MUTEX_INITIALIZER };
This is not allowed in our code as we build with win32 threads which
initialize mutexes completely different. Why do you want to initialize
it here anyway ...
+
typedef struct _qemuEmulatorCache qemuEmulatorCache;
typedef qemuEmulatorCache* qemuEmulatorCachePtr;
struct _qemuEmulatorCache {
@@ -206,9 +212,17 @@ qemuEmulatorCachedInfoGet(enum qemuCapsP
const char *binary,
const char *arch);
+int
+qemuCapsCacheInit(void)
+{
+return virMutexInit(qemuEmulatorCacheMutex);
+}
+
if you have created this function?
static void
qemuEmulatorCachedInfoRelease(qemuEmulatorCachePtr emulator ATTRIBUTE_UNUSED)
-{ }
+{
+virMutexUnlock(qemuEmulatorCacheMutex);
+}
/* Feature flags for the architecture info */
static const struct qemu_feature_flags const arch_info_i686_flags [] = {
@@ -1769,6 +1783,8 @@ qemuEmulatorCachedInfoGet(enum qemuCapsP
bool alreadyCached;
int i;
+virMutexLock(qemuEmulatorCacheMutex);
+
if (stat(binary, st) != 0) {
char ebuf[1024];
VIR_INFO(Failed to stat emulator %s : %s,
Index: libvirt-0.9.10/src/qemu/qemu_driver.c
===
--- libvirt-0.9.10.orig/src/qemu/qemu_driver.c
+++ libvirt-0.9.10/src/qemu/qemu_driver.c
@@ -585,6 +585,9 @@ qemudStartup(int privileged) {
if (qemuSecurityInit(qemu_driver) 0)
goto error;
+if (qemuCapsCacheInit() 0)
+goto error;
+
if ((qemu_driver-caps = qemuCreateCapabilities(NULL,
qemu_driver)) == NULL)
goto error;
Index: libvirt-0.9.10/src/qemu/qemu_capabilities.h
===
--- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.h
+++ libvirt-0.9.10/src/qemu/qemu_capabilities.h
@@ -139,6 +139,8 @@ void qemuCapsClear(virBitmapPtr caps,
bool qemuCapsGet(virBitmapPtr caps,
enum qemuCapsFlags flag);
+int qemuCapsCacheInit(void);
+
virCapsPtr qemuCapsInit(virCapsPtr old_caps);
int qemuCapsProbeMachineTypes(const char *binary,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] graphics: Cleanup port policy
Even though we say in documentation setting (tls-)port to -1 is legacy
compat style for enabling autoport, we're roughly doing this for VNC.
However, in case of SPICE auto enable autoport iff both port tlsPort
are equal -1 as documentation says autoport plays with both.
---
src/conf/domain_conf.c | 30 --
src/conf/domain_conf.h |5 +
src/qemu/qemu_command.c |2 +-
src/qemu/qemu_process.c | 33 -
4 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b185fe7..d142512 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5929,6 +5929,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
VIR_FREE(port);
goto error;
}
+/* Legacy compat syntax, used -1 for auto-port */
+if (def-data.rdp.port == -1)
+def-data.rdp.autoport = 1;
+
VIR_FREE(port);
} else {
def-data.rdp.port = 0;
@@ -5936,14 +5940,15 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
}
if ((autoport = virXMLPropString(node, autoport)) != NULL) {
-if (STREQ(autoport, yes)) {
-if (flags VIR_DOMAIN_XML_INACTIVE)
-def-data.rdp.port = 0;
+if (STREQ(autoport, yes))
def-data.rdp.autoport = 1;
-}
+
VIR_FREE(autoport);
}
+if (def-data.rdp.autoport (flags VIR_DOMAIN_XML_INACTIVE))
+def-data.rdp.port = 0;
+
if ((replaceUser = virXMLPropString(node, replaceUser)) != NULL) {
if (STREQ(replaceUser, yes)) {
def-data.rdp.replaceUser = 1;
@@ -6009,16 +6014,21 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
}
if ((autoport = virXMLPropString(node, autoport)) != NULL) {
-if (STREQ(autoport, yes)) {
-if (flags VIR_DOMAIN_XML_INACTIVE) {
-def-data.spice.port = 0;
-def-data.spice.tlsPort = 0;
-}
+if (STREQ(autoport, yes))
def-data.spice.autoport = 1;
-}
VIR_FREE(autoport);
}
+if (def-data.spice.port == -1 def-data.spice.tlsPort == -1) {
+/* Legacy compat syntax, used -1 for auto-port */
+def-data.spice.autoport = 1;
+}
+
+if (def-data.spice.autoport (flags VIR_DOMAIN_XML_INACTIVE)) {
+def-data.spice.port = 0;
+def-data.spice.tlsPort = 0;
+}
+
def-data.spice.keymap = virXMLPropString(node, keymap);
if (virDomainGraphicsAuthDefParseXML(node, def-data.spice.auth,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6fc307e..6da22f4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1183,6 +1183,11 @@ struct _virDomainGraphicsListenDef {
};
struct _virDomainGraphicsDef {
+/* Port value discipline:
+ * Value -1 is legacy syntax indicating that it should be auto-allocated.
+ * Value 0 means port wasn't specified in XML at all.
+ * Positive value is actual port number given in XML.
+ */
int type;
union {
struct {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 996763c..b6dd1f1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5375,7 +5375,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(opt, port=%u, def-graphics[0]-data.spice.port);
-if (def-graphics[0]-data.spice.tlsPort) {
+if (def-graphics[0]-data.spice.tlsPort 0) {
if (!driver-spiceTLS) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(spice TLS port set in XML configuration,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1ac892f..ef311d1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3169,28 +3169,35 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
vm-def-graphics[0]-data.vnc.port = port;
-} else if (vm-def-graphics[0]-type ==
VIR_DOMAIN_GRAPHICS_TYPE_SPICE
- vm-def-graphics[0]-data.spice.autoport) {
-int port = qemuProcessNextFreePort(driver, QEMU_VNC_PORT_MIN);
-int tlsPort = -1;
-if (port 0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-%s, _(Unable to find an unused SPICE
port));
-goto cleanup;
+} else if (vm-def-graphics[0]-type ==
VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+int port = -1;
+if (vm-def-graphics[0]-data.spice.autoport ||
+vm-def-graphics[0]-data.spice.port == -1) {
+port = qemuProcessNextFreePort(driver, QEMU_VNC_PORT_MIN);
+
+if (port 0) {
+
[libvirt] [PATCH 3/4] storage: Implement jobs for storage driver
Simply, when we are about to take an action which might take ages,
like allocating new volumes, wiping, etc. increment a counter of
jobs in pool object and unlock it. We don't want to hold the pool
locked during long term actions.
---
src/conf/storage_conf.c | 12 ++
src/conf/storage_conf.h | 22 +++-
src/libvirt_private.syms |1 +
src/storage/storage_driver.c | 284 +++---
4 files changed, 246 insertions(+), 73 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index bdf6218..e378ceb 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -251,6 +251,8 @@ virStorageVolDefFree(virStorageVolDefPtr def) {
if (!def)
return;
+virMutexDestroy(def-lock);
+ignore_value(virCondDestroy(def-job.cond));
VIR_FREE(def-name);
VIR_FREE(def-key);
@@ -978,6 +980,16 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
return NULL;
}
+if (virMutexInit(ret-lock) 0) {
+virReportSystemError(errno, %s, _(Cannot init mutex));
+goto cleanup;
+}
+
+if (virCondInit(ret-job.cond) 0) {
+virReportSystemError(errno, %s, _(Cannot init cond));
+goto cleanup;
+}
+
ret-name = virXPathString(string(./name), ctxt);
if (ret-name == NULL) {
virStorageReportError(VIR_ERR_XML_ERROR,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 1ef9295..481c806 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -83,6 +83,25 @@ struct _virStorageVolTarget {
};
+enum virStorageVolJob {
+VIR_STORAGE_VOL_JOB_NONE = 0, /* no job */
+VIR_STORAGE_VOL_JOB_BUILD,
+VIR_STORAGE_VOL_JOB_DELETE,
+VIR_STORAGE_VOL_JOB_REFRESH,
+VIR_STORAGE_VOL_JOB_WIPE,
+VIR_STORAGE_VOL_JOB_RESIZE,
+VIR_STORAGE_VOL_JOB_DOWNLOAD,
+VIR_STORAGE_VOL_JOB_UPLOAD,
+
+VIR_STORAGE_VOL_JOB_LAST
+};
+
+struct virStorageVolJobObj {
+virCond cond;
+enum virStorageVolJob active;
+unsigned long long start;
+};
+
typedef struct _virStorageVolDef virStorageVolDef;
typedef virStorageVolDef *virStorageVolDefPtr;
struct _virStorageVolDef {
@@ -90,7 +109,8 @@ struct _virStorageVolDef {
char *key;
int type; /* virStorageVolType enum */
-unsigned int building;
+virMutex lock;
+struct virStorageVolJobObj job;
unsigned long long allocation; /* bytes */
unsigned long long capacity; /* bytes */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1f55f5d..fef9d5a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,6 +565,7 @@ virFDStreamOpen;
virFDStreamConnectUNIX;
virFDStreamOpenFile;
virFDStreamCreateFile;
+virFDStreamSetInternalCloseCb;
# hash.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 66811ce..7f3dfcd 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -48,6 +48,7 @@
#include virfile.h
#include fdstream.h
#include configmake.h
+#include virtime.h
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -65,6 +66,111 @@ static void storageDriverUnlock(virStorageDriverStatePtr
driver)
}
static void
+storageResetJob(virStorageVolDefPtr vol)
+{
+struct virStorageVolJobObj *job = vol-job;
+
+job-active = VIR_STORAGE_VOL_JOB_NONE;
+job-start = 0;
+}
+
+/* wait max. 30 seconds for job ackquire */
+#define STORAGE_JOB_WAIT_TIME (1000ull * 30)
+
+static int
+storageBeginJobInternal(virStorageDriverStatePtr driver ATTRIBUTE_UNUSED,
+virStoragePoolObjPtr pool,
+bool pool_locked,
+virStorageVolDefPtr vol,
+enum virStorageVolJob job)
+{
+unsigned long long now;
+unsigned long long then;
+
+if (virTimeMillisNow(now) 0)
+return -1;
+
+then = now + STORAGE_JOB_WAIT_TIME;
+
+while (vol-job.active != VIR_STORAGE_VOL_JOB_NONE) {
+if (virCondWaitUntil(vol-job.cond, vol-lock, then) 0)
+goto error;
+}
+
+VIR_DEBUG(Starting job %d, job);
+storageResetJob(vol);
+vol-job.active = job;
+vol-job.start = now;
+
+if (pool_locked) {
+pool-asyncjobs++;
+virStoragePoolObjUnlock(pool);
+}
+
+return 0;
+
+error:
+if (errno == ETIMEDOUT)
+virStorageReportError(VIR_ERR_OPERATION_TIMEOUT, %s,
+ _(cannot acquire state change lock));
+else
+virReportSystemError(errno, %s,
+ _(cannot acquire job mutex));
+if (pool_locked)
+virStoragePoolObjLock(pool);
+return -1;
+}
+
+static int
+storageBeginJob(virStorageDriverStatePtr driver,
+virStorageVolDefPtr vol,
+enum virStorageVolJob job)
+{
+return storageBeginJobInternal(driver, NULL, false, vol, job);
+}
+
+static int
+storageBeginJobWithPool(virStorageDriverStatePtr driver,
+
[libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob
This API can be used to terminate long running jobs
on a volume like its building, resizing, wiping.
Moreover, like virDomainAbortJob() calling this API
will block until job has either completed or aborted.
---
include/libvirt/libvirt.h.in |3 ++
src/driver.h |5
src/libvirt.c| 49 ++
src/libvirt_public.syms |1 +
src/remote/remote_driver.c |1 +
src/remote/remote_protocol.x |8 ++-
src/remote_protocol-structs |5
7 files changed, 71 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 7d41642..77ec3f0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2513,6 +2513,9 @@ int virStorageVolResize
(virStorageVolPtr vol,
unsigned long long
capacity,
unsigned int flags);
+int virStorageVolAbortJob (virStorageVolPtr vol,
+ unsigned int flags);
+
/**
* virKeycodeSet:
diff --git a/src/driver.h b/src/driver.h
index 03d249b..7845b06 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1314,6 +1314,10 @@ typedef int
unsigned int flags);
typedef int
+(*virDrvStorageVolAbortJob) (virStorageVolPtr vol,
+ unsigned int flags);
+
+typedef int
(*virDrvStoragePoolIsActive)(virStoragePoolPtr pool);
typedef int
(*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool);
@@ -1377,6 +1381,7 @@ struct _virStorageDriver {
virDrvStorageVolResize volResize;
virDrvStoragePoolIsActive poolIsActive;
virDrvStoragePoolIsPersistent poolIsPersistent;
+virDrvStorageVolAbortJobvolAbortJob;
};
# ifdef WITH_LIBVIRTD
diff --git a/src/libvirt.c b/src/libvirt.c
index e916aa0..8ce3234 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13343,6 +13343,55 @@ error:
}
/**
+ * virStorageVolAbortJob:
+ * @vol: pointer to storage volume
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Requests that the current background job be aborted at the soonest
+ * opportunity. This will block until the job has either completed,
+ * or aborted.
+ *
+ * Returns: 0 in case of success
+ * -1 otherwise
+ */
+int
+virStorageVolAbortJob(virStorageVolPtr vol,
+ unsigned int flags)
+{
+virConnectPtr conn;
+VIR_DEBUG(vol=%p flags=%x, vol, flags);
+
+virResetLastError();
+
+if (!VIR_IS_STORAGE_VOL(vol)) {
+virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+
+conn = vol-conn;
+
+if (conn-flags VIR_CONNECT_RO) {
+ virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ goto error;
+}
+
+if (conn-storageDriver conn-storageDriver-volAbortJob) {
+int ret;
+ret = conn-storageDriver-volAbortJob(vol, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(vol-conn);
+return -1;
+}
+
+/**
* virNodeNumOfDevices:
* @conn: pointer to the hypervisor connection
* @cap: capability name
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 46c13fb..cd3e2a6 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -532,6 +532,7 @@ LIBVIRT_0.9.10 {
LIBVIRT_0.9.11 {
global:
virDomainPMWakeup;
+virStorageVolAbortJob;
} LIBVIRT_0.9.10;
# define new API here using predicted next version number
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 031167a..3534ac0 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5013,6 +5013,7 @@ static virStorageDriver storage_driver = {
.volResize = remoteStorageVolResize, /* 0.9.10 */
.poolIsActive = remoteStoragePoolIsActive, /* 0.7.3 */
.poolIsPersistent = remoteStoragePoolIsPersistent, /* 0.7.3 */
+.volAbortJob = remoteStorageVolAbortJob, /* 0.9.11 */
};
static virSecretDriver secret_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 4d845e7..014eade 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1754,6 +1754,11 @@ struct remote_storage_vol_resize_args {
unsigned int flags;
};
+struct remote_storage_vol_abort_job_args {
+remote_nonnull_storage_vol vol;
+unsigned int flags;
+};
+
/* Node driver calls: */
struct remote_node_num_of_devices_args {
@@ -2765,7 +2770,8 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SET_METADATA = 264, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_METADATA = 265, /* autogen autogen */
[libvirt] [PATCH 0/4] Introduce jobs for storage driver
Disk operations can take ages to finish. Therefore users might want to abort such job, e.g. because host is going down for maintenance. This patch set is trying to allow this kind of behaviour. The inspiration was taken from qemu driver. How it works: An API that is known to run for a long time, e.g. volume allocation, wiping, have to find a pool which volume belongs to. If the pool was found it is locked. During this time the storage driver is locked. Now the API is preparing for taking the operation. Ideally, it would be sufficient to have only volume locked, but it requires far more changes to code. Just before the API is about to take the long term action, storageBeginJob[WithPool] is called. This wait on a condition specific for a volume. Similar to QEMU driver, wait is limited in time. Timeout is set to 30 seconds. Upon successful job acquire, a counter in the pool is incremented so the pool is prevented from destroy, undef; Then the pool is unlocked. After API finishes it's work, it calls storageEndJob[WithPool] which reset job structure in the volume, re-locks the pool, decrement the counter and broadcasts on the condition. Meanwhile, if the long term action is done internally, it should be done in a cycle so we can check if job abort wasn't signalized. If it is done by external program, we can virCommandAbort() it. Several improvements can be made, but I think this is good for start. NB, I've implemented job aborting for the both APIs using streams, however wasn't very successful with aborting a stream from the daemon site. Michal Privoznik (4): storage: Introduce virStorageVolAbortJob virsh: Expose virStorageVolAbortJob storage: Implement jobs for storage driver storage: Implement virStorageVolAbortJob include/libvirt/libvirt.h.in |3 + src/conf/storage_conf.c | 12 ++ src/conf/storage_conf.h | 29 +++- src/driver.h |5 + src/libvirt.c| 49 + src/libvirt_private.syms |4 + src/libvirt_public.syms |1 + src/remote/remote_driver.c |1 + src/remote/remote_protocol.x |8 +- src/remote_protocol-structs |5 + src/storage/storage_backend.c| 53 +++--- src/storage/storage_backend_fs.c |8 +- src/storage/storage_driver.c | 408 ++ src/storage/storage_driver.h |7 + tools/virsh.c| 39 15 files changed, 516 insertions(+), 116 deletions(-) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] virsh: Expose virStorageVolAbortJob
via new virsh command 'vol-jobabort'. Currently, it accepts
only volume specification as argument.
---
tools/virsh.c | 39 +++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 630b77f..00668ff 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12497,6 +12497,44 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
return true;
}
+/*
+ * vol-jobabort command
+ */
+static const vshCmdInfo info_vol_jobabort[] = {
+{help, N_(abort active volume job)},
+{desc, N_(Aborts the currently running volume job)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_vol_jobabort[] = {
+{vol, VSH_OT_DATA, VSH_OFLAG_REQ, N_(volume name or key)},
+{pool, VSH_OT_STRING, 0, N_(pool name or uuid)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdVolJobAbort(vshControl *ctl, const vshCmd *cmd)
+{
+virStorageVolPtr vol;
+bool ret = false;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return false;
+
+if (!(vol = vshCommandOptVol(ctl, cmd, vol, pool, NULL))) {
+return false;
+}
+
+if (virStorageVolAbortJob(vol, 0) 0)
+goto cleanup;
+
+vshPrint(ctl, _(Job successfully aborted));
+ret = true;
+
+cleanup:
+virStorageVolFree(vol);
+return ret;
+}
/*
* secret-define command
@@ -17239,6 +17277,7 @@ static const vshCmdDef storageVolCmds[] = {
{vol-download, cmdVolDownload, opts_vol_download, info_vol_download, 0},
{vol-dumpxml, cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml, 0},
{vol-info, cmdVolInfo, opts_vol_info, info_vol_info, 0},
+{vol-jobabort, cmdVolJobAbort, opts_vol_jobabort, info_vol_jobabort, 0},
{vol-key, cmdVolKey, opts_vol_key, info_vol_key, 0},
{vol-list, cmdVolList, opts_vol_list, info_vol_list, 0},
{vol-name, cmdVolName, opts_vol_name, info_vol_name, 0},
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] storage: Implement virStorageVolAbortJob
This implies breaking up some jobs into cycles during which
we check for job abortion request. The virStorageVolAbortJob
API will then just set request and wait until job is released.
If a job was, however, interrupted it should fail with
VIR_ERR_OPERATION_ABORTED error.
---
src/conf/storage_conf.h |7 ++
src/libvirt_private.syms |3 +
src/storage/storage_backend.c| 53 +---
src/storage/storage_backend_fs.c |8 ++-
src/storage/storage_driver.c | 128 -
src/storage/storage_driver.h |7 ++
6 files changed, 162 insertions(+), 44 deletions(-)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 481c806..ee25e34 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -28,6 +28,7 @@
# include util.h
# include storage_encryption_conf.h
# include threads.h
+# include command.h
# include libxml/tree.h
@@ -100,6 +101,12 @@ struct virStorageVolJobObj {
virCond cond;
enum virStorageVolJob active;
unsigned long long start;
+
+bool abort_requested; /* abort was requested */
+virCommandPtr cmd; /* if we are running external program,
+ store pointer here too, so we can
+ virCommandAbort it if necessary */
+virStreamPtr stream;/* stream to abort */
};
typedef struct _virStorageVolDef virStorageVolDef;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fef9d5a..cec7a94 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1017,6 +1017,9 @@ virStorageVolDefParseFile;
virStorageVolDefParseNode;
virStorageVolDefParseString;
+# storage_driver.h
+virStorageVolJobSetCommand;
+virStorageVolJobSetStream;
# storage_encryption_conf.h
virStorageEncryptionFormat;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index caac2f8..2e82c03 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -53,6 +53,7 @@
#include internal.h
#include secret_conf.h
#include uuid.h
+#include storage_driver.h
#include storage_file.h
#include storage_backend.h
#include logging.h
@@ -106,8 +107,6 @@ static virStorageBackendPtr backends[] = {
NULL
};
-static int track_allocation_progress = 0;
-
enum {
TOOL_QEMU_IMG,
TOOL_KVM_IMG,
@@ -202,6 +201,15 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
goto cleanup;
}
+
+if (vol-job.abort_requested ||
+inputvol-job.abort_requested) {
+virStorageReportError(VIR_ERR_OPERATION_ABORTED, %s,
+ _(Job abort requested));
+ret = -1;
+goto cleanup;
+}
+
} while ((amtleft -= interval) 0);
}
@@ -325,34 +333,31 @@ createRawFile(int fd, virStorageVolDefPtr vol,
}
if (remain) {
-if (track_allocation_progress) {
+while (remain) {
+/* Allocate in chunks of 512MiB: big-enough chunk
+ * size and takes approx. 9s on ext3. A progress
+ * update every 9s is a fair-enough trade-off
+ */
+unsigned long long bytes = 512 * 1024 * 1024;
-while (remain) {
-/* Allocate in chunks of 512MiB: big-enough chunk
- * size and takes approx. 9s on ext3. A progress
- * update every 9s is a fair-enough trade-off
- */
-unsigned long long bytes = 512 * 1024 * 1024;
-
-if (bytes remain)
-bytes = remain;
-if (safezero(fd, vol-allocation - remain, bytes) 0) {
-ret = -errno;
-virReportSystemError(errno, _(cannot fill file '%s'),
- vol-target.path);
-goto cleanup;
-}
-remain -= bytes;
-}
-} else { /* No progress bars to be shown */
-if (safezero(fd, 0, remain) 0) {
+if (bytes remain)
+bytes = remain;
+if (safezero(fd, vol-allocation - remain, bytes) 0) {
ret = -errno;
virReportSystemError(errno, _(cannot fill file '%s'),
vol-target.path);
goto cleanup;
}
-}
+remain -= bytes;
+if (vol-job.abort_requested) {
+virStorageReportError(VIR_ERR_OPERATION_ABORTED,
+ %s,
+ _(Job abort requested));
+ret = -1;
+goto cleanup;
+}
+}
}
if (fsync(fd) 0) {
@@ -782,6 +787,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
goto cleanup;
cmd = virCommandNew(create_tool);
+virStorageVolJobSetCommand(vol,
Re: [libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob
On 13.03.2012 15:48, Daniel P. Berrange wrote: On Tue, Mar 13, 2012 at 03:35:29PM +0100, Michal Privoznik wrote: This API can be used to terminate long running jobs on a volume like its building, resizing, wiping. Moreover, like virDomainAbortJob() calling this API will block until job has either completed or aborted. --- include/libvirt/libvirt.h.in |3 ++ src/driver.h |5 src/libvirt.c| 49 ++ src/libvirt_public.syms |1 + src/remote/remote_driver.c |1 + src/remote/remote_protocol.x |8 ++- src/remote_protocol-structs |5 7 files changed, 71 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7d41642..77ec3f0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2513,6 +2513,9 @@ int virStorageVolResize (virStorageVolPtr vol, unsigned long long capacity, unsigned int flags); +int virStorageVolAbortJob (virStorageVolPtr vol, + unsigned int flags); + No, virStorageVolGetJobInfo() API to go with it ? IMHO we should have both, so we mirror the virDomain job API design. Regards, Daniel yeah, virStorageVolGetJobInfo() is one of the improvements I'm mentioning in cover letter. But I've decided to not implement it for now as another huge bunch of code would have to be rewritten make this patch set unbearable big. But if it is a show stopper I can rewrite and post v2. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Reverse condition in qemuDomainCheckDiskPresence
With current code, we pass true iff domain is cold booting. However,
if disk is inaccessible and startupPolicy for that disk is set to
'requisite' we have to fail iff cold booting.
---
src/qemu/qemu_domain.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f8b7c96..625c595 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1703,7 +1703,7 @@ qemuDomainSetFakeReboot(struct qemud_driver *driver,
int
qemuDomainCheckDiskPresence(struct qemud_driver *driver,
virDomainObjPtr vm,
-bool start_with_state)
+bool cold_boot)
{
int ret = -1;
int i;
@@ -1738,7 +1738,7 @@ qemuDomainCheckDiskPresence(struct qemud_driver *driver,
break;
case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
-if (!start_with_state) {
+if (cold_boot) {
virReportSystemError(errno,
_(cannot access file '%s'),
disk-src);
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: A bit smarter attach-disk
On 15.03.2012 11:17, Osier Yang wrote: Detects the file type of source path if no --sourcetype and driver is specified, instead of always set the disk type as block. --- tools/virsh.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: A bit smarter attach-disk
On 15.03.2012 10:13, Osier Yang wrote:
Detects the file type of source path if no --sourcetype and
driver is specified, instead of always set the disk type as
block.
And previous virCommandOptString ensures source is not NULL,
remove the conditional checking.
---
tools/virsh.c | 14 +-
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index d45a4c9..3b845ac 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -14428,6 +14428,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
const char *stype = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *xml;
+struct stat st;
if (!vshConnectionUsability(ctl, ctl-conn))
goto cleanup;
@@ -14458,8 +14459,12 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
}
if (!stype) {
-if (driver (STREQ(driver, file) || STREQ(driver, tap)))
+if (driver (STREQ(driver, file) || STREQ(driver, tap))) {
isFile = true;
+} else {
+if (!stat(source, st))
+isFile = S_ISREG(st.st_mode) ? true : false;
I think S_ISREG() would be sufficient here. But that's just cosmetic.
+}
} else if (STREQ(stype, file)) {
isFile = true;
} else if (STRNEQ(stype, block)) {
@@ -14497,10 +14502,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (driver || subdriver || cache)
virBufferAddLit(buf, /\n);
-if (source)
-virBufferAsprintf(buf, source %s='%s'/\n,
- (isFile) ? file : dev,
- source);
+virBufferAsprintf(buf, source %s='%s'/\n,
+ (isFile) ? file : dev,
+ source);
virBufferAsprintf(buf, target dev='%s'/\n, target);
if (mode)
virBufferAsprintf(buf, %s/\n, mode);
However this looks bad. As written in commend just below
virCommandOptString(, source):
/* Allow empty string as a placeholder that implies no source, for
* use in adding a cdrom drive with no disk. */
if (!*source)
source = NULL;
That means:
virsh attach-disk domain dummy
make source NULL. Therefore you want to check source != NULL in the
first chunk too (okay, not as strict as here, since passing NULL to
stat() makes it fail, but it's clean coding style what matters too).
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuDomainDetachPciDiskDevice: Free allocated cgroup
This function potentially allocates new virCgroup but never frees it. --- src/qemu/qemu_hotplug.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e088a49..f1c1283 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1621,6 +1621,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, ret = 0; cleanup: +virCgroupFree(cgroup); VIR_FREE(drivestr); return ret; } -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainDetachPciDiskDevice: Free allocated cgroup
On 15.03.2012 13:04, Peter Krempa wrote: On 03/15/2012 11:49 AM, Michal Privoznik wrote: This function potentially allocates new virCgroup but never frees it. --- ACK. Peter Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] qemu: Wire up virDomainSuspendForDuration API
On 16.03.2012 07:52, Osier Yang wrote:
On 03/16/2012 07:11 AM, Eric Blake wrote:
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
This makes use of QEMU guest agent to implement the
virDomainSuspendForDuration API.
---
src/qemu/qemu_driver.c | 93
1 files changed, 93 insertions(+), 0 deletions(-)
+
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+%s, _(domain is not running));
+goto cleanup;
+}
Same question as for quiesce: putting the guest into S3 will only work
if the agent can respond, so checking merely for active is not enough.
If the guest is active but paused, then we can't talk to the agent to
issue the request. Having the common guest agent code check for this
condition will prevent the scenario of:
guest is paused
issue the pm suspend, but it times out
Actually I encounted the problem when playing dompmsuspend,
the dompmsuspend command hangs forever, so I tried to
suspend the guest with pm-suspend inside guest directly,
and then dompmwakeup returns quickly with success, however,
the guest wasn't waken up actually.
So there are at least two problems here:
1) dompmsuspend hangs
2) dompmwakeup returns success, while the guest wasn't waken
up actally.
For 1), I tried to create a patch with using guest agent command
guest-ping to ping the guest agent first before executing the
guest-suspend-* commands, and hope it could return quickly if the
guest agent isn't available or something else which cause the guest
agent doesn't response, and thus we could quit quickly, but no luck,
the guest-ping hangs too.
This was my approach as well:
https://www.redhat.com/archives/libvir-list/2012-February/msg00055.html
Although I've used waiting with timeout; therefore making any fail
harmless. Because if we timeout on guest-sync which doesn't change the
state of guest, we don't issue any subsequent state-changing command.
I should reorder my TODO list and post the patch again. The sooner the
better.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu_agent: Issue guest-sync prior to every command
If we issue guest command and GA is not running, the issuing thread
will block endlessly. We can check for GA presence by issuing
guest-sync with unique ID (timestamp). We don't want to issue real
command as even if GA is not running, once it is started, it process
all commands written to GA socket.
---
diff to v1:
- don't keep list of issued IDs because it's pointless
Some background on this:
I've intended to switch to new guest-sync-delimited and use
older guest-sync for older GA. However, since we don't use
stream base implementation but use new line as delimiter for
GA responses we don't need GA to issue sentinel byte 0xFF for us:
http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol
Moreover, since we are using guest-sync just for detecting GA, it is
not necessary to use sentinel byte at all:
http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03278.html
src/qemu/qemu_agent.c | 134 ++--
1 files changed, 128 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index a17d025..ee82fae 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -39,6 +39,7 @@
#include virterror_internal.h
#include json.h
#include virfile.h
+#include virtime.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -316,6 +317,7 @@ qemuAgentIOProcessLine(qemuAgentPtr mon,
{
virJSONValuePtr obj = NULL;
int ret = -1;
+unsigned long long id;
VIR_DEBUG(Line [%s], line);
@@ -340,6 +342,20 @@ qemuAgentIOProcessLine(qemuAgentPtr mon,
obj = NULL;
ret = 0;
} else {
+/* If we've received something like:
+ * {return: 1234}
+ * it is likely that somebody started GA
+ * which is now processing our previous
+ * guest-sync commands. Check if this is
+ * the case and don't report an error but
+ * return silently.
+ */
+if (virJSONValueObjectGetNumberUlong(obj, return, id) == 0) {
+VIR_DEBUG(Ignoring delayed reply to guest-sync: %llu, id);
+ret = 0;
+goto cleanup;
+}
+
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_(Unexpected JSON reply '%s'), line);
}
@@ -813,11 +829,28 @@ void qemuAgentClose(qemuAgentPtr mon)
qemuAgentUnlock(mon);
}
+#define QEMU_AGENT_WAIT_TIME (1000ull * 5)
+/**
+ * qemuAgentSend:
+ * @mon: Monitor
+ * @msg: Message
+ * @timeout: use timeout?
+ *
+ * Send @msg to agent @mon.
+ * Wait max QEMU_AGENT_WAIT_TIME for agent
+ * to reply.
+ *
+ * Returns: 0 on success,
+ * -2 on timeout,
+ * -1 otherwise
+ */
static int qemuAgentSend(qemuAgentPtr mon,
- qemuAgentMessagePtr msg)
+ qemuAgentMessagePtr msg,
+ bool timeout)
{
int ret = -1;
+unsigned long long now, then;
/* Check whether qemu quit unexpectedly */
if (mon-lastError.code != VIR_ERR_OK) {
@@ -827,13 +860,26 @@ static int qemuAgentSend(qemuAgentPtr mon,
return -1;
}
+if (timeout) {
+if (virTimeMillisNow(now) 0)
+return -1;
+then = now + QEMU_AGENT_WAIT_TIME;
+}
+
mon-msg = msg;
qemuAgentUpdateWatch(mon);
while (!mon-msg-finished) {
-if (virCondWait(mon-notify, mon-lock) 0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
-_(Unable to wait on monitor condition));
+if ((timeout virCondWaitUntil(mon-notify, mon-lock, then) 0)
||
+(!timeout virCondWait(mon-notify, mon-lock) 0)) {
+if (errno == ETIMEDOUT) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(Guest agent not available for now));
+ret = -2;
+} else {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(Unable to wait on monitor condition));
+}
goto cleanup;
}
}
@@ -855,6 +901,78 @@ cleanup:
}
+/**
+ * qemuAgentGuestSync:
+ * @mon: Monitor
+ *
+ * Send guest-sync with unique ID
+ * and wait for reply. If we get one, check if
+ * received ID is equal to given.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise
+ */
+static int
+qemuAgentGuestSync(qemuAgentPtr mon)
+{
+int ret = -1;
+int send_ret;
+unsigned long long id, id_ret;
+qemuAgentMessage sync_msg;
+
+memset(sync_msg, 0, sizeof sync_msg);
+
+if (virTimeMillisNow(id) 0)
+return -1;
+
+if (virAsprintf(sync_msg.txBuffer,
+{\execute\:\guest-sync\,
+\arguments\:{\id\:%llu}}, id) 0) {
+virReportOOMError();
+return -1;
+}
+
+sync_msg.txLength = strlen(sync_msg.txBuffer);
+
+VIR_DEBUG(Sending guest-sync command with ID: %llu, id);
+
+
Re: [libvirt] [PATCH v2] Fixed NULL pointer check
On 19.03.2012 08:55, Martin Kletzander wrote:
This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
src/util/conf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
{
virConfEntryPtr cur;
+if (conf == NULL)
+return NULL;
+
cur = conf-entries;
while (cur != NULL) {
if ((cur-name != NULL)
Looking good, but I think we should revert 59d0c9801c1ab then.
In addition - maybe we can set ATTRIBUTE_RETURN_CHECK to this function.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] snapshot: make quiesce a bit safer
On 16.03.2012 21:49, Eric Blake wrote: If a guest is paused, we were silently ignoring the quiesce flag, which results in unclean snapshots, contrary to the intent of the flag. Since we can't quiesce without guest agent support, we should instead fail if the guest is not running. Meanwhile, if we attempt a quiesce command, but the guest agent doesn't respond, and we time out, we may have left the command pending on the guest's queue, and when the guest resumes parsing commands, it will freeze even though our command is no longer around to issue a thaw. To be safe, we must _always_ pair every quiesce call with a counterpart thaw, even if the quiesce call failed due to a timeout, so that if a guest wakes up and starts processing a command backlog, it will not get stuck in a frozen state. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive): Always issue thaw after a quiesce, even if quiesce failed. (qemuDomainSnapshotFSThaw): Add a parameter. --- See also: https://bugzilla.redhat.com/show_bug.cgi?id=804210 https://www.redhat.com/archives/libvir-list/2012-March/msg00708.html src/qemu/qemu_driver.c | 51 +-- 1 files changed, 36 insertions(+), 15 deletions(-) ACK this and the follow up patch as well. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] Fixed NULL pointer check
On 19.03.2012 11:05, Martin Kletzander wrote: This patch fixes a NULL pointer check that was causing SegFault on some specific configurations. It also reverts commit 59d0c9801c1ab that was checking for this value in one place. --- v3: - added revert of 59d0c9801c1ab that's not needed anymore - (comment) ATTRIBUTE_RETURN_CHECK not added as it is not used with other functions defined in this file and the commit could be confusing modifying all of them together. However, there should be one patch to fix all these at once if possible v2: - removed parenthesis around return value src/libvirt.c |3 +-- src/util/conf.c |5 - 2 files changed, 5 insertions(+), 3 deletions(-) I've tweaked the patch subject (prepended with 'virConfGetValue: ' for easier git-log) and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] python: Avoid memory leaks on libvirt_virNodeGetMemoryStats
On 17.02.2012 09:15, a...@redhat.com wrote: From: Alex Jia a...@redhat.com Detected by valgrind. Leaks are introduced in commit 17c7795. * python/libvirt-override.c (libvirt_virNodeGetMemoryStats): fix memory leaks and improve codes return value. For details, please see the following link: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=770944 Signed-off-by: Alex Jia a...@redhat.com --- python/libvirt-override.c | 46 ++-- 1 files changed, 31 insertions(+), 15 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: drop a painfully long gnulib test
On 21.03.2012 00:14, Eric Blake wrote: On machines with massive amounts of CPUs, the gnulib 'test-lock' could take minutes, or even appear to deadlock, because of timing interactions between multiple cores. See https://bugzilla.redhat.com/show_bug.cgi?id=797284. For precedence, note that iwhd has done the same: https://lists.gnu.org/archive/html/bug-gnulib/2012-01/msg00311.html We can re-enable things if gnulib ever analyzes and improves the situation. * bootstrap.conf (gnulib_tool_option_extras): Avoid lock-tests. --- Personally, I haven't run into the problem on my 2-core laptop; where do I sign up for one of the 2048-core machines in that BZ? :) But since I haven't seen it, I can't push this under the trivial rule, so I'll wait for a review. bootstrap.conf |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 03da267..c6f7fd9 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -174,6 +174,7 @@ gnulib_tool_option_extras=\ --with-tests\ --makefile-name=gnulib.mk\ --avoid=pt_chown\ + --avoid=lock-tests\ local_gl_dir=gnulib/local ACK, if user's system can't lock properly he would experience problems regardless running libvirt. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: Build activeUsbHostdevs list on process reconnect
If the daemon is restarted it will loose list of active
USB devices assigned to active domains. Therefore we need
to rebuild this list on qemuProcessReconnect().
---
src/qemu/qemu_hostdev.c | 40
src/qemu/qemu_hostdev.h |2 ++
src/qemu/qemu_process.c |3 +++
3 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 6ce2421..b45acec 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -157,6 +157,46 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver
*driver,
return 0;
}
+int
+qemuUpdateActiveUsbHostdevs(struct qemud_driver *driver,
+virDomainDefPtr def)
+{
+virDomainHostdevDefPtr hostdev = NULL;
+int i;
+
+if (!def-nhostdevs)
+return 0;
+
+for (i = 0; i def-nhostdevs; i++) {
+usbDevice *usb = NULL;
+hostdev = def-hostdevs[i];
+
+if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+continue;
+if (hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+continue;
+
+usb = usbGetDevice(hostdev-source.subsys.u.usb.bus,
+ hostdev-source.subsys.u.usb.device);
+if (!usb) {
+VIR_WARN(Unable to reattach USB device %03d.%03d on domain %s,
+ hostdev-source.subsys.u.usb.bus,
+ hostdev-source.subsys.u.usb.device,
+ def-name);
+continue;
+}
+
+usbDeviceSetUsedBy(usb, def-name);
+
+if (usbDeviceListAdd(driver-activeUsbHostdevs, usb) 0) {
+usbFreeDevice(usb);
+return -1;
+}
+}
+
+return 0;
+}
+
static int
qemuDomainHostdevPciSysfsPath(virDomainHostdevDefPtr hostdev, char
**sysfs_path)
{
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 173e4f4..371630a 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -29,6 +29,8 @@
int qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
virDomainDefPtr def);
+int qemuUpdateActiveUsbHostdevs(struct qemud_driver *driver,
+virDomainDefPtr def);
int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
const char *name,
const unsigned char *uuid,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0e768fe..ae1be0f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3057,6 +3057,9 @@ qemuProcessReconnect(void *opaque)
goto error;
}
+if (qemuUpdateActiveUsbHostdevs(driver, obj-def) 0)
+goto error;
+
if (qemuProcessUpdateState(driver, obj) 0)
goto error;
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] qemu: Improve USB devices list handling
This patch set fixes two bugs: https://bugzilla.redhat.com/show_bug.cgi?id=806449 https://bugzilla.redhat.com/show_bug.cgi?id=743671 tl;dr - we are only adding to the list, removing only on detach-device. Need to remove on qemuProcessStop too. And need to rebuild the list on qemuProcessReconnect. Michal Privoznik (3): qemu: Don't leak temporary list of USB devices qemu: Delete USB devices used by domain on stop qemu: Build activeUsbHostdevs list on process reconnect src/qemu/qemu_hostdev.c | 112 +-- src/qemu/qemu_hostdev.h |2 + src/qemu/qemu_process.c |3 + 3 files changed, 113 insertions(+), 4 deletions(-) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: Don't leak temporary list of USB devices
and add debug message when adding USB device
to the list of active devices.
---
src/qemu/qemu_hostdev.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 329e3fc..d4d7461 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -559,10 +559,7 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
hostdev-source.subsys.u.usb.product);
if (!usb)
-return -1;
-
-hostdev-source.subsys.u.usb.bus = usbDeviceGetBus(usb);
-hostdev-source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+goto cleanup;
if ((tmp = usbDeviceListFind(driver-activeUsbHostdevs, usb))) {
const char *other_name = usbDeviceGetUsedBy(tmp);
@@ -579,6 +576,9 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
goto cleanup;
}
+hostdev-source.subsys.u.usb.bus = usbDeviceGetBus(usb);
+hostdev-source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+
if (usbDeviceListAdd(list, usb) 0) {
usbFreeDevice(usb);
goto cleanup;
@@ -594,6 +594,9 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
for (i = 0; i usbDeviceListCount(list); i++) {
tmp = usbDeviceListGet(list, i);
usbDeviceSetUsedBy(tmp, name);
+
+VIR_DEBUG(Adding %03d.%03d dom=%s to activeUsbHostdevs,
+ usbDeviceGetBus(tmp), usbDeviceGetDevno(tmp), name);
if (usbDeviceListAdd(driver-activeUsbHostdevs, tmp) 0) {
usbFreeDevice(tmp);
goto inactivedevs;
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: Delete USB devices used by domain on stop
To prevent assigning one USB device to two domains,
we keep a list of assigned USB devices. On domain
startup - qemuProcessStart() - we insert devices
used by domain into the list but remove them only
on detach-device. Devices are, however, released
on qemuProcessReconnect() as well.
---
src/qemu/qemu_hostdev.c | 61 +++
1 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index d4d7461..6ce2421 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -755,6 +755,64 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver
*driver,
pciDeviceListFree(pcidevs);
}
+static void
+qemuDomainReAttachHostUsbDevices(struct qemud_driver *driver,
+ const char *name,
+ virDomainHostdevDefPtr *hostdevs,
+ int nhostdevs)
+{
+int i;
+
+for (i = 0; i nhostdevs; i++) {
+virDomainHostdevDefPtr hostdev = hostdevs[i];
+usbDevice *usb, *tmp;
+const char *used_by = NULL;
+
+if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+continue;
+if (hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+continue;
+
+usb = usbGetDevice(hostdev-source.subsys.u.usb.bus,
+ hostdev-source.subsys.u.usb.device);
+
+if (!usb) {
+VIR_WARN(Unable to reattach USB device %03d.%03d on domain %s,
+ hostdev-source.subsys.u.usb.bus,
+ hostdev-source.subsys.u.usb.device,
+ name);
+continue;
+}
+
+/* Delete only those USB devices which belongs
+ * to domain @name because qemuProcessStart() might
+ * have failed because USB device is already taken.
+ * Therefore we want to steal only those devices from
+ * the list which were taken by @name */
+
+tmp = usbDeviceListFind(driver-activeUsbHostdevs, usb);
+if (!tmp) {
+VIR_WARN(Unable to find device %03d.%03d
+ in list of active USB devices,
+ hostdev-source.subsys.u.usb.bus,
+ hostdev-source.subsys.u.usb.device);
+usbFreeDevice(usb);
+continue;
+}
+
+used_by = usbDeviceGetUsedBy(tmp);
+if (STREQ_NULLABLE(used_by, name)) {
+VIR_DEBUG(Removing %03d.%03d dom=%s from activeUsbHostdevs,
+ hostdev-source.subsys.u.usb.bus,
+ hostdev-source.subsys.u.usb.device,
+ name);
+
+usbDeviceListDel(driver-activeUsbHostdevs, tmp);
+}
+
+usbFreeDevice(usb);
+}
+}
void qemuDomainReAttachHostDevices(struct qemud_driver *driver,
virDomainDefPtr def)
@@ -764,4 +822,7 @@ void qemuDomainReAttachHostDevices(struct qemud_driver
*driver,
qemuDomainReAttachHostdevDevices(driver, def-name, def-hostdevs,
def-nhostdevs);
+
+qemuDomainReAttachHostUsbDevices(driver, def-name, def-hostdevs,
+ def-nhostdevs);
}
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Avoid entering monitor with locked driver
On 27.03.2012 14:19, Jiri Denemark wrote: This avoids possible deadlock of the qemu driver in case a domain is begin migrated (in Begin phase) and unrelated connection to qemu driver is closed at the right time. I checked all callers of qemuDomainCheckEjectableMedia() and they are calling this function with qemu driver locked. --- src/qemu/qemu_hotplug.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt PATCHv7 1/1] add DHCP snooping
Stefan was faster, but some nits I've spotted in addition to his. On 26.03.2012 22:25, David L Stevens wrote: This patch adds DHCP snooping support to libvirt. The learning method for IP addresses is specified by setting the ip_learning variable to one of any [default] (existing IP learning code), none (static only addresses) or dhcp (DHCP snooping). Active leases are saved in a lease file and reloaded on restart or HUP. Changes since v6: - replace pthread_cancel() with synchronous cancelation method Signed-off-by: David L Stevens dlstev...@us.ibm.com --- docs/formatnwfilter.html.in| 17 + src/Makefile.am|2 + src/nwfilter/nwfilter_dhcpsnoop.c | 1139 src/nwfilter/nwfilter_dhcpsnoop.h | 38 ++ src/nwfilter/nwfilter_driver.c |6 + src/nwfilter/nwfilter_gentech_driver.c | 59 ++- 6 files changed, 1248 insertions(+), 13 deletions(-) create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 9cb7644..ad10765 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -2189,6 +2189,23 @@ br/br/ In case a VM is resumed after suspension or migrated, IP address detection will be restarted. + br/br/ + Variable iip_learning/i may be used to specify + the IP address learning method. Valid values are iany/i, idhcp/i, + or inone/i. The default value is iany/i, meaning that libvirt + may use any packet to determine the address in use by a VM. A value of + idhcp/i specifies that libvirt should only honor DHCP server-assigned + addresses with valid leases. If iip_learning/i is set to inone/i, + libvirt does not do address learning and referencing iIP/i without + assigning it an explicit value is an error. + br/br/ + Use of iip_learning=dhcp/i (DHCP snooping) provides additional + anti-spoofing security, especially when combined with a filter allowing + only trusted DHCP servers to assign addresses. If the DHCP lease expires, + the VM will no longer be able to use the IP address until it acquires a + new, valid lease from a DHCP server. If the VM is migrated, it must get + a new valid DHCP lease to use an IP address (e.g., by + bringing the VM interface down and up again). /p h3a name=nwflimitsmigrVM Migration/a/h3 diff --git a/src/Makefile.am b/src/Makefile.am index a2aae9d..4382caf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -509,6 +509,8 @@ NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ nwfilter/nwfilter_gentech_driver.c \ nwfilter/nwfilter_gentech_driver.h \ + nwfilter/nwfilter_dhcpsnoop.c \ + nwfilter/nwfilter_dhcpsnoop.h \ nwfilter/nwfilter_ebiptables_driver.c \ nwfilter/nwfilter_ebiptables_driver.h \ nwfilter/nwfilter_learnipaddr.c \ diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c new file mode 100644 index 000..8e5dcc5 --- /dev/null +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -0,0 +1,1139 @@ +/* + * nwfilter_dhcpsnoop.c: support for DHCP snooping used by a VM + * on an interface + * + * Copyright (C) 2011 IBM Corp. + * Copyright (C) 2011 David L Stevens + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: David L Stevens dlstev...@us.ibm.com + * Based in part on work by Stefan Berger stef...@us.ibm.com + */ + +#include config.h + +#ifdef HAVE_LIBPCAP +#include pcap.h +#endif + +#include fcntl.h +#include sys/ioctl.h +#include signal.h + +#include arpa/inet.h +#include net/ethernet.h +#include netinet/ip.h +#include netinet/udp.h +#include net/if.h +#include net/if_arp.h +#include intprops.h + +#include
Re: [libvirt] [libvirt PATCHv7 1/1] add DHCP snooping
Round two On 26.03.2012 22:25, David L Stevens wrote: This patch adds DHCP snooping support to libvirt. The learning method for IP addresses is specified by setting the ip_learning variable to one of any [default] (existing IP learning code), none (static only addresses) or dhcp (DHCP snooping). Active leases are saved in a lease file and reloaded on restart or HUP. Changes since v6: - replace pthread_cancel() with synchronous cancelation method Signed-off-by: David L Stevens dlstev...@us.ibm.com --- docs/formatnwfilter.html.in| 17 + src/Makefile.am|2 + src/nwfilter/nwfilter_dhcpsnoop.c | 1139 src/nwfilter/nwfilter_dhcpsnoop.h | 38 ++ src/nwfilter/nwfilter_driver.c |6 + src/nwfilter/nwfilter_gentech_driver.c | 59 ++- 6 files changed, 1248 insertions(+), 13 deletions(-) create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 9cb7644..ad10765 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -2189,6 +2189,23 @@ br/br/ In case a VM is resumed after suspension or migrated, IP address detection will be restarted. + br/br/ + Variable iip_learning/i may be used to specify + the IP address learning method. Valid values are iany/i, idhcp/i, + or inone/i. The default value is iany/i, meaning that libvirt + may use any packet to determine the address in use by a VM. A value of + idhcp/i specifies that libvirt should only honor DHCP server-assigned + addresses with valid leases. If iip_learning/i is set to inone/i, + libvirt does not do address learning and referencing iIP/i without + assigning it an explicit value is an error. + br/br/ + Use of iip_learning=dhcp/i (DHCP snooping) provides additional + anti-spoofing security, especially when combined with a filter allowing + only trusted DHCP servers to assign addresses. If the DHCP lease expires, + the VM will no longer be able to use the IP address until it acquires a + new, valid lease from a DHCP server. If the VM is migrated, it must get + a new valid DHCP lease to use an IP address (e.g., by + bringing the VM interface down and up again). /p h3a name=nwflimitsmigrVM Migration/a/h3 diff --git a/src/Makefile.am b/src/Makefile.am index a2aae9d..4382caf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -509,6 +509,8 @@ NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ nwfilter/nwfilter_gentech_driver.c \ nwfilter/nwfilter_gentech_driver.h \ + nwfilter/nwfilter_dhcpsnoop.c \ + nwfilter/nwfilter_dhcpsnoop.h \ nwfilter/nwfilter_ebiptables_driver.c \ nwfilter/nwfilter_ebiptables_driver.h \ nwfilter/nwfilter_learnipaddr.c \ diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c new file mode 100644 index 000..8e5dcc5 --- /dev/null +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -0,0 +1,1139 @@ +/* + * nwfilter_dhcpsnoop.c: support for DHCP snooping used by a VM + * on an interface + * + * Copyright (C) 2011 IBM Corp. + * Copyright (C) 2011 David L Stevens + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: David L Stevens dlstev...@us.ibm.com + * Based in part on work by Stefan Berger stef...@us.ibm.com + */ + +#include config.h + +#ifdef HAVE_LIBPCAP +#include pcap.h +#endif + +#include fcntl.h +#include sys/ioctl.h +#include signal.h Do we need signal.h? It seems unused to me. + +#include arpa/inet.h +#include net/ethernet.h +#include netinet/ip.h +#include netinet/udp.h +#include net/if.h +#include net/if_arp.h +#include intprops.h Why do we include this
Re: [libvirt] [PATCH] libvirt: Fix build err
On 29.03.2012 14:55, ailvpen...@gmail.com wrote: From: Zhou Peng ailvpen...@gmail.com virNetDevMacVLanRestartWithVPortProfile is omitted in src/libvirt_private.syms, which causes link err. --- src/libvirt_private.syms |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c50940b..97fec2f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1288,6 +1288,7 @@ virNetDevMacVLanCreate; virNetDevMacVLanDelete; virNetDevMacVLanCreateWithVPortProfile; virNetDevMacVLanDeleteWithVPortProfile; +virNetDevMacVLanRestartWithVPortProfile; # virnetdevopenvswitch.h I've tweaked the commit message a bit and pushed. Thanks! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: silence recent syntax check violations
On 30.03.2012 05:23, Eric Blake wrote:
An upstream gnulib bug meant that some of our syntax checks
weren't being run. Fix up our offenders before we upgrade to
a newer gnulib.
* src/util/virnetdevtap.c (virNetDevTapCreate): Use flags.
* tests/lxcxml2xmltest.c (mymain): Strip useless ().
---
The gnulib bug was here:
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html
I tested this by temporarily using the fixed gnulib maint.mk.
Pushing under the trivial rule. I can't call it a build breaker,
because it won't break the build without a gnulib update.
I'm reluctant to update the .gnulib submodule this late in the
game without some review, as we've had bad luck with a submodule
update after the rc1 build in previous releases, so I'm saving
that for another day. Besides, I'm waiting for a review of a
patch that fixes ssize_t for mingw, and it isn't worth a gnulib
update without that fix.
src/util/virnetdevtap.c |7 +--
tests/lxcxml2xmltest.c |4 ++--
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 0b3ac46..717b6ac 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -129,12 +129,14 @@ virNetDevProbeVnetHdr(int tapfd)
*/
int virNetDevTapCreate(char **ifname,
int *tapfd,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
int fd;
struct ifreq ifr;
int ret = -1;
+virCheckFlags(VIR_NETDEV_TAP_CREATE_VNET_HDR, -1);
+
This is incomplete; networkStartNetworkVirtual() pass
VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE here. If a gnulib check is
causing fail, we should disable that check. Testing for flags at public
API is something that *have to* be done; omitting test at this low layer
and setting ATTRIBUTE_UNUSED is something that is intentional and
therefore is harmless.
Therefore we should either exclude src/util/* from checking, or drop
u_int flags completely here as they are unused after all. Okay, we can
also make virCheckFlags complete.
if ((fd = open(/dev/net/tun, O_RDWR)) 0) {
virReportSystemError(errno, %s,
_(Unable to open /dev/net/tun, is tun module
loaded?));
@@ -237,8 +239,9 @@ cleanup:
#else /* ! TUNSETIFF */
int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
int *tapfd ATTRIBUTE_UNUSED,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
+virCheckFlags(0, -1);
virReportSystemError(ENOSYS, %s,
_(Unable to create TAP devices on this platform));
However, this is even worse. Instead of throwing ENOSYS - the only
correct error here - we can throw some spurious error about unsupported
flags. This makes me lean towards excluding src/util/* from the check
causing trouble.
return -1;
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index 558bd01..6a87939 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -99,7 +99,7 @@ mymain(void)
int ret = 0;
if ((caps = testLXCCapsInit()) == NULL)
-return (EXIT_FAILURE);
+return EXIT_FAILURE;
# define DO_TEST_FULL(name, is_different, inactive) \
do {\
@@ -124,7 +124,7 @@ mymain(void)
virCapabilitiesFree(caps);
-return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
+return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
VIRT_TEST_MAIN(mymain)
ACK to these two changes.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Prevent crash of libvirtd without channel target name
On 30.03.2012 11:44, Alex Jia wrote: * src/conf/domain_conf.c (virDomainChannelDefCheckABIStability): avoid crashing libvirtd due to derefing a NULL pointer. For details, please see bug: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=808371 Signed-off-by: Alex Jia a...@redhat.com --- src/conf/domain_conf.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virnetdevtap: Don't check for flags in virNetDevTapCreateFlags
With latest gnulib we are checking even the lowest level functions
whether they check flags. Moreover, we are shadowing the real error
on system without TUNSETIFF support.
---
cfg.mk |2 +-
src/util/virnetdevtap.c |5 +
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index c3de533..d44c8d2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -741,7 +741,7 @@ exclude_file_name_regexp--sc_avoid_write = \
exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
-exclude_file_name_regexp--sc_flags_usage = ^docs/
+exclude_file_name_regexp--sc_flags_usage = ^(docs/|src/util/virnetdevtap\.c$$)
exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
^src/rpc/gendispatch\.pl$$
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 717b6ac..54f76bd 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -129,14 +129,12 @@ virNetDevProbeVnetHdr(int tapfd)
*/
int virNetDevTapCreate(char **ifname,
int *tapfd,
- unsigned int flags)
+ unsigned int flags ATTRIBUTE_UNUSED)
{
int fd;
struct ifreq ifr;
int ret = -1;
-virCheckFlags(VIR_NETDEV_TAP_CREATE_VNET_HDR, -1);
-
if ((fd = open(/dev/net/tun, O_RDWR)) 0) {
virReportSystemError(errno, %s,
_(Unable to open /dev/net/tun, is tun module
loaded?));
@@ -241,7 +239,6 @@ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
int *tapfd ATTRIBUTE_UNUSED,
unsigned int flags)
{
-virCheckFlags(0, -1);
virReportSystemError(ENOSYS, %s,
_(Unable to create TAP devices on this platform));
return -1;
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virnetdevtap: Don't check for flags in virNetDevTapCreateFlags
On 30.03.2012 15:04, Eric Blake wrote: On 03/30/2012 05:49 AM, Michal Privoznik wrote: With latest gnulib we are checking even the lowest level functions whether they check flags. Moreover, we are shadowing the real error on system without TUNSETIFF support. --- cfg.mk |2 +- src/util/virnetdevtap.c |5 + 2 files changed, 2 insertions(+), 5 deletions(-) @@ -241,7 +239,6 @@ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED, int *tapfd ATTRIBUTE_UNUSED, unsigned int flags) Missing the ATTRIBUTE_UNUSED here. ACK with that fix. Yeah, my in brain git-revert has some defects :) Pushed, thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: set default name for SPICE agent channel when generating command
On 30.03.2012 09:50, Laine Stump wrote:
commit b0e2bb33 set a default value for the SPICE agent channel by
inserting it during parsing of the channel XML. That method of setting
a default is problematic because it makes a format/parse roundtrip
unclean, and experience with setting other values as a side effect of
parsing has led to headaches (e.g. automatically setting a MAC address
in the parser when one isn't specified in the input XML).
This patch reverts commit b0e2bb33 and replaces it with the alternate
implementation of simply inserting the default value in the
appropriate place on the qemu commandline when no value is provided.
---
(Playing the devil's advocate on my own patch for a moment - one
advantage of Christophe's method of setting the default is that if we
use that object somewhere else in libvirt's code in the future, the
value will be set even if the XML left it unset, but with my method we
will have to check for a NULL name and replace it with the default
value anywhere we want to use it. So I won't say that either method is
definitely the proper way to go, but will just present this
alternative and see if someone else has an even stronger opinion than
me :-)
(BTW, I think I've decided while typing this message that, if we
decide to change from the original method of setting default to this
new method, the change should be pushed as two separate patches - one
reverting the original, and another adding the new. It's too close to
morning for me to take the time to do that right now, though.)
src/conf/domain_conf.c |7 ---
src/qemu/qemu_command.c |6 +++---
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24e10e6..ea558bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5248,13 +5248,6 @@ virDomainChrDefParseXML(virCapsPtr caps,
goto error;
} else {
def-source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT;
-if (!def-target.name) {
-def-target.name = strdup(com.redhat.spice.0);
-if (!def-target.name) {
-virReportOOMError();
-goto error;
-}
-}
}
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..50b1e6d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3457,9 +3457,9 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) {
virBufferAsprintf(buf, ,chardev=char%s,id=%s,
dev-info.alias, dev-info.alias);
-if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
-dev-target.name) {
-virBufferAsprintf(buf, ,name=%s, dev-target.name);
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL) {
+virBufferAsprintf(buf, ,name=%s, dev-target.name
+ ? dev-target.name : com.redhat.spice.0);
}
} else {
virBufferAsprintf(buf, ,id=%s, dev-info.alias);
ACK
Maybe it's worth squashing this in:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..3a14727 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3444,8 +3444,7 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr
dev,
if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
dev-source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC
-dev-target.name
-STRNEQ(dev-target.name, com.redhat.spice.0)) {
+STRNEQ_NULLABLE(dev-target.name, com.redhat.spice.0)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported spicevmc target name '%s'),
dev-target.name);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu_agent: Issue guest-sync prior to every command
On 30.03.2012 16:31, Jiri Denemark wrote: On Fri, Mar 16, 2012 at 17:35:09 +0100, Michal Privoznik wrote: If we issue guest command and GA is not running, the issuing thread will block endlessly. We can check for GA presence by issuing guest-sync with unique ID (timestamp). We don't want to issue real command as even if GA is not running, once it is started, it process all commands written to GA socket. --- diff to v1: - don't keep list of issued IDs because it's pointless Some background on this: I've intended to switch to new guest-sync-delimited and use older guest-sync for older GA. However, since we don't use stream base implementation but use new line as delimiter for GA responses we don't need GA to issue sentinel byte 0xFF for us: http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol Moreover, since we are using guest-sync just for detecting GA, it is not necessary to use sentinel byte at all: http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03278.html src/qemu/qemu_agent.c | 134 ++-- 1 files changed, 128 insertions(+), 6 deletions(-) ACK, but don't forget to run make syntax-check before pushing to fix your violation of recently added sizeof() rule. Thanks pushed. Anyway, there is a small chance the guest-agent will crash after sending us a response to guest-sync but before it can process the real command. Current code will just hang waiting for the reply. We were discussing this issue with Michal and came up with a possible solution: - use timeouts for all ga commands - any code sending ga command has to register a callback which will be used in case the command takes longer than timeout; in that case the caller already exited with error and the callback will make sure to undo any changes the command (which we thought was never acted upon) made; e.g., it will unfreeze guest's filesystem after we didn't get a reply to freeze in time - when a new command is about to be issued, guest-sync is called (already done) and getting reply to it removes any callbacks since we know it won't be processed - anytime the callback is present, we may report that guest agent is not responding to us using domcontrol (if possible) or using a new api Jirka Yeah, I'll post follow up patch once we're after release. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Start of freeze for libvirt-0.9.11 and availability of rc1
On 01.04.2012 03:35, Laine Stump wrote: On 03/31/2012 07:56 AM, Daniel Veillard wrote: I just made the second release candidate ftp://libvirt.org/libvirt/libvirt-0.9.11-rc2.tar.gz along with the rpms and tagged the git tree. It seems to work okay in my limited testing. If all goes well I would probably push on Tuesday, A gentoo user reported in irc (#virt on irc.oftc.net) earlier today that builds were failing for him on a fresh checkout from git: feniksa hi, when i compile libvirt on my gentoo i get warning: cc1: warning: unrecognized command line option -Wno-suggest-attribute=const cc1: warning: unrecognized command line option -Wno-suggest-attribute=pure feniksa gcc (Gentoo 4.5.3-r2 p1.1, pie-0.4.7) 4.5.3 feniksa last part of conf: http://pastebin.com/gnT4Y2Ba and compile log http://pastebin.com/FcSwshzS feniksa laine: 2 weeks ago i didn't see this warning. Now i pull from master sources and get this warning. But, this is only because i am using gcc 4.5.3 (this is latest gcc version in gentoo OS). I haven't looked at any of this code before, but it looks to me like ql_WARN_ADD() in gnulib must not be doing the right thing, and is adding the option even though the compiler apparently doesn't support it. This probably didn't crop up before because full warnings weren't turned on in the build until March 27 in commit 851117bd. That commit does things different for builds of git trees vs. tarballs, so I don't know if this failure will affect builds of release tarballs (I asked feniksa to try a build of the rc2 tarball, but he hasn't responded yet). I use x86_64-pc-linux-gnu-4.6.2 and don't see those warnings. However, configure is checking whether compiler handles those parameters. So maybe it's a autotools bug? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Clarify escape sequence
Currently, we put no strains on escape sequence possibly leaving users
with console that cannot be terminated. However, not all ASCII
characters can be used as escape sequence. Only those falling in
@ - _ can be; implement and document this constraint.
---
tools/console.c |3 ++-
tools/virsh.c | 13 -
tools/virsh.pod |3 ++-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/console.c b/tools/console.c
index ca226c3..0f30b95 100644
--- a/tools/console.c
+++ b/tools/console.c
@@ -34,6 +34,7 @@
# include errno.h
# include unistd.h
# include signal.h
+# include c-ctype.h
# include internal.h
# include console.h
@@ -292,7 +293,7 @@ static char
vshGetEscapeChar(const char *s)
{
if (*s == '^')
-return CONTROL(s[1]);
+return CONTROL(c_islower(s[1]) ? c_toupper(s[1]) : s[1]);
return *s;
}
diff --git a/tools/virsh.c b/tools/virsh.c
index 1ed2dda..cfdd040 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -19879,6 +19879,16 @@ vshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED)
vshPrint(ctl, \n);
}
+static bool
+vshAllowedEscapeChar(char c)
+{
+/* Allowed escape characters:
+ * a-z A-Z @ [ \ ] ^ _
+ */
+return ('a' = c c = 'z') ||
+('@' = c c = '_');
+}
+
/*
* argv[]: virsh [options] [command]
*
@@ -19942,7 +19952,8 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
case 'e':
len = strlen(optarg);
-if ((len == 2 *optarg == '^') ||
+if ((len == 2 *optarg == '^'
+ vshAllowedEscapeChar(optarg[1])) ||
(len == 1 *optarg != '^')) {
ctl-escapeChar = optarg;
} else {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index d4971a3..a60e667 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -95,7 +95,8 @@ Output elapsed time information for each command.
=item B-e, B--escape Istring
Set alternative escape sequence for Iconsole command. By default,
-telnet's B^] is used.
+telnet's B^] is used. Allowed characters when using hat notation are:
+alphabetic character, @, [, ], \, ^, _.
=back
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Improve USB devices list handling
On 26.03.2012 17:39, Michal Privoznik wrote: This patch set fixes two bugs: https://bugzilla.redhat.com/show_bug.cgi?id=806449 https://bugzilla.redhat.com/show_bug.cgi?id=743671 tl;dr - we are only adding to the list, removing only on detach-device. Need to remove on qemuProcessStop too. And need to rebuild the list on qemuProcessReconnect. Michal Privoznik (3): qemu: Don't leak temporary list of USB devices qemu: Delete USB devices used by domain on stop qemu: Build activeUsbHostdevs list on process reconnect src/qemu/qemu_hostdev.c | 112 +-- src/qemu/qemu_hostdev.h |2 + src/qemu/qemu_process.c |3 + 3 files changed, 113 insertions(+), 4 deletions(-) Ping? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] I need help for use virDomainInterfaceStats
On 02.04.2012 20:56, siddharth wrote: Фёдор Пранович pr.fedor at gmail.com writes: Hi I want use the virDomainInterfaceStats for get statistic of domain, but I don't now how get path parametr. Please help me! Thanks. Please Help me Too !! Basically, it's string(/domain/devices/interface[n]/target/@dev) where n is index of the interface you wanna get statistics for. For example, if I have this in XML (virDomainGetXMLDesc()): ... interface type='network' mac address='52:54:00:57:f5:50'/ source network='default'/ target dev='vnet0'/ model type='virtio'/ filterref filter='no-ip-spoofing' parameter name='ip_learning' value='dhcp'/ /filterref alias name='net0'/ address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/ /interface interface type='direct' mac address='52:54:00:16:e6:5c'/ source dev='eth0' mode='bridge'/ target dev='macvtap0'/ model type='virtio'/ alias name='net1'/ address type='pci' domain='0x' bus='0x00' slot='0x08' function='0x0'/ /interface ... I would query for stats: virDomainInterfaceStats(dom, vnet0, ...); virDomainInterfaceStats(dom, macvtap0, ...); Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Need Help To Destroy Virtual Machines
On 03.04.2012 16:42, Ali Raza Memon wrote: Hello...! I am using libvirt-php for controlling my xen virtual machines. For this I have installed libvirt-php along with xampp server. Now I need to create a web based controll panel application which shows me running vms, and some options like Shutdown, Pause, Create, Destroy. My aim is, when I click Destroy button the selected vm should be destroyed. For this I have used the following php-script: ?php $conn=libvirt_connect(xen:///); $name=libvirt_domain_lookup_by_id($conn,4); // Here '4' is the id of my running vm $dest=libvirt_domain_destroy($name); echo $dest; ? When I execute this it shows me following warning: *Warning: libvirt_domain_destroy() [function.libvirt-domain-destroy]: operation virDomainDestroy forbidden for read only access in /opt/lampp/htdocs/xampp/xen/test.php on line 5 * So, Could you please help me to solve this? ! Thank You ! You need to pass credentials array: $credentials = array(VIR_CRED_AUTHNAME='root',VIR_CRED_PASSPHRASE='super_secret_password'); $conn = libvirt_connect(xen:///, FALSE, $credentials); // FALSE means you want read-write connection You can find examplese here: http://phplibvirt.cybersales.cz/doc/function.libvirt-connect.html Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Clarify escape sequence
On 03.04.2012 16:39, Eric Blake wrote:
On 04/03/2012 07:10 AM, Michal Privoznik wrote:
Currently, we put no strains on escape sequence possibly leaving users
with console that cannot be terminated. However, not all ASCII
characters can be used as escape sequence. Only those falling in
@ - _ can be; implement and document this constraint.
vshGetEscapeChar(const char *s)
{
if (*s == '^')
-return CONTROL(s[1]);
+return CONTROL(c_islower(s[1]) ? c_toupper(s[1]) : s[1]);
Unlike toupper(), c_toupper() is safe even on non-alphabetic characters
(that's part of why gnulib wrote c-ctype.h); you can shorten this to:
('@' = c c = '_');
ACK with nit fixed.
Fixed and pushed. Thanks.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] news.html.in: Fix /br void tag
Void elements should be written with slash *after* the tag name, not before, so they are not confused with ending tags. --- Pushing under trivial rule. Produced by: :%s/\/br/br\//g command, so if breaks something, blame vim :) docs/news.html.in | 642 ++-- 1 files changed, 321 insertions(+), 321 deletions(-) diff --git a/docs/news.html.in b/docs/news.html.in index 68b2d3a..eb9c3ec 100644 --- a/docs/news.html.in +++ b/docs/news.html.in @@ -1,4 +1,4 @@ -?xml version=1.0? +?xml version=1.0? html head meta http-equiv=Content-Type content=text/html; charset=utf-8 / @@ -11,336 +11,336 @@ and check the a href=http://libvirt.org/git/?p=libvirt.git;a=log;GIT log/a h30.9.11: Apr 3 2012/h3 ul li Features:br/ - Add support for the suspend event (Osier Yang),/br - Add support for event tray moved of removable disks (Osier Yang),/br - qemu: Support numad (Osier Yang),/br - cpustats: API, improvements and qemu support (KAMEZAWA Hiroyuki and Eric Blake),/br - qemu: support type='hostdev' network devices at domain start (Laine Stump),/br - Introduce virDomainPMWakeup API (Michal Privoznik),/br - network: support Open vSwitch (Ansis Atteka),/br - a number of snapshot improvements (Eric Blake)/br + Add support for the suspend event (Osier Yang),br/ + Add support for event tray moved of removable disks (Osier Yang),br/ + qemu: Support numad (Osier Yang),br/ + cpustats: API, improvements and qemu support (KAMEZAWA Hiroyuki and Eric Blake),br/ + qemu: support type='hostdev' network devices at domain start (Laine Stump),br/ + Introduce virDomainPMWakeup API (Michal Privoznik),br/ + network: support Open vSwitch (Ansis Atteka),br/ + a number of snapshot improvements (Eric Blake)br/ /li - li Portability:/br - build: fix build on cygwin (Eric Blake),/br - build: fix mingw ssize_t, syntax check (Eric Blake),/br - Disable build of commandhelper amp; ssh on Win32 (Daniel P. Berrange),/br - build: avoid 'devname' for BSD (Eric Blake),/br - build: avoid frame size error when building without -O2 (Laine Stump),/br - spec: Add missed dependancy for numad (Osier Yang),/br - util: fix build mingw (and all non-linux) build failure (Laine Stump),/br - Build error on OSX in src/util/virnetlink.c (Duncan Rance),/br - Fix build after commit e3ba4025 (Jim Fehlig),/br - build: Fix build with dtrace + apparmor (Jiri Denemark),/br - build: fix output of pid values (Eric Blake),/br - avoid global variable shadowed (Hu Tao),/br - lxc: Cleaner fix for compilation without SELinux (Martin Kletzander),/br - Fix compilation on MacOS X (Lincoln Myers)/br + li Portability:br/ + build: fix build on cygwin (Eric Blake),br/ + build: fix mingw ssize_t, syntax check (Eric Blake),br/ + Disable build of commandhelper amp; ssh on Win32 (Daniel P. Berrange),br/ + build: avoid 'devname' for BSD (Eric Blake),br/ + build: avoid frame size error when building without -O2 (Laine Stump),br/ + spec: Add missed dependancy for numad (Osier Yang),br/ + util: fix build mingw (and all non-linux) build failure (Laine Stump),br/ + Build error on OSX in src/util/virnetlink.c (Duncan Rance),br/ + Fix build after commit e3ba4025 (Jim Fehlig),br/ + build: Fix build with dtrace + apparmor (Jiri Denemark),br/ + build: fix output of pid values (Eric Blake),br/ + avoid global variable shadowed (Hu Tao),br/ + lxc: Cleaner fix for compilation without SELinux (Martin Kletzander),br/ + Fix compilation on MacOS X (Lincoln Myers)br/ /li - li Documentation:/br - snapshot: fix virsh docs (Eric Blake),/br - Expand docs for timer tick policy (Daniel P. Berrange),/br - Add documentation for new attribute tray of disk target (Osier Yang),/br - Clarify virsh freecell manpage entry (Dave Allan),/br - fix typo (Zhou Peng),/br - Clarify what documentation is being referenced (Dave Allan),/br - Minor docs fix (Martin Kletzander),/br - libvirt: fix comment typo (Alex Jia),/br - fix usage example on setting log levels (Eric Blake),/br - use correct terminology for 1024 bytes (Eric Blake),/br - Fix typo (Osier Yang),/br - Fix typo in domain XML documentation (Christophe Fergeau),/br - storage: fix typo (Michal Privoznik),/br - comments wiping supported algorithms (Alex Jia),/br - Fix libvirt name in qemu commandline namespace URL (Michal Privoznik),/br - virsh: Break long lines in virsh.pod (Osier Yang),/br - Update bug reporting page (Dave Allan),/br - lib: Fix function documentation for virConnectListDomains (Peter Krempa),/br - virsh: Fix docs for list command (Peter Krempa)/br + li Documentation:br/ + snapshot: fix virsh docs (Eric Blake),br
Re: [libvirt] [PATCH] news.html.in: Fix /br void tag
On 03.04.2012 17:38, Daniel P. Berrange wrote: On Tue, Apr 03, 2012 at 09:33:43AM -0600, Eric Blake wrote: On 04/03/2012 09:25 AM, Michal Privoznik wrote: Void elements should be written with slash *after* the tag name, not before, so they are not confused with ending tags. --- Pushing under trivial rule. Produced by: :%s/\/br/br\//g command, so if breaks something, blame vim :) docs/news.html.in | 642 ++-- 1 files changed, 321 insertions(+), 321 deletions(-) diff --git a/docs/news.html.in b/docs/news.html.in index 68b2d3a..eb9c3ec 100644 --- a/docs/news.html.in +++ b/docs/news.html.in @@ -1,4 +1,4 @@ -?xml version=1.0? +?xml version=1.0? And there's the first broken change. :( Further more, I wonder why our xmllint check did not complain about either this problem, or the original problem Daniel Because the flow is like this: %.html.tmp: %.html.in %.html: %.html.tmp And we are using xmllint for validation only when creating %.html not %html.tmp; Moreover, xsltproc we are using for generating %.html.tmp omitted badly formated tags. In other words: Lorem ipsum /br\n in %.html.in got translated into: Lorem ipsum \n in %html.tmp which is compliant to XML. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Improve USB devices list handling
On 03.04.2012 15:12, Michal Privoznik wrote: On 26.03.2012 17:39, Michal Privoznik wrote: This patch set fixes two bugs: https://bugzilla.redhat.com/show_bug.cgi?id=806449 https://bugzilla.redhat.com/show_bug.cgi?id=743671 tl;dr - we are only adding to the list, removing only on detach-device. Need to remove on qemuProcessStop too. And need to rebuild the list on qemuProcessReconnect. Michal Privoznik (3): qemu: Don't leak temporary list of USB devices qemu: Delete USB devices used by domain on stop qemu: Build activeUsbHostdevs list on process reconnect src/qemu/qemu_hostdev.c | 112 +-- src/qemu/qemu_hostdev.h |2 + src/qemu/qemu_process.c |3 + 3 files changed, 113 insertions(+), 4 deletions(-) Ping? Thanks for review; fixed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_ga: Don't overwrite errors on FSThaw
We can tell qemuDomainSnapshotFSThaw if we want it to report errors or
not. However, if we don't want to and an error has been already set by
previous qemuReportError() we must keep copy of that error not just a
pointer to it. Otherwise, it get overwritten if FSThaw reports an error.
---
src/qemu/qemu_driver.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dd79973..0880f51 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9636,16 +9636,13 @@ qemuDomainSnapshotFSThaw(struct qemud_driver *driver,
qemuDomainObjEnterAgent(driver, vm);
if (!report)
-err = virGetLastError();
+err = virSaveLastError();
thawed = qemuAgentFSThaw(priv-agent);
-if (!report) {
-if (err)
-virResetError(err);
-else
-virResetLastError();
-}
+if (!report)
+virSetError(err);
qemuDomainObjExitAgent(driver, vm);
+virFreeError(err);
return thawed;
}
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_ga: Don't overwrite errors on FSThaw
On 06.04.2012 13:38, Jiri Denemark wrote: On Fri, Apr 06, 2012 at 12:58:02 +0200, Michal Privoznik wrote: We can tell qemuDomainSnapshotFSThaw if we want it to report errors or not. However, if we don't want to and an error has been already set by previous qemuReportError() we must keep copy of that error not just a pointer to it. Otherwise, it get overwritten if FSThaw reports an error. --- src/qemu/qemu_driver.c | 11 --- 1 files changed, 4 insertions(+), 7 deletions(-) ACK Jirka Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuOpenFile: Don't force chown on NFS
If dynamic_ownership is off and we are creating a file on NFS
we force chown. This will fail as chown/chmod are not supported
on NFS. However, with no dynamic_ownership we are not required
to do any chown.
---
src/qemu/qemu_driver.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d9e35be..1b55eb1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2429,6 +2429,7 @@ qemuOpenFile(struct qemud_driver *driver, const char
*path, int oflags,
bool bypass_security = false;
unsigned int vfoflags = 0;
int fd = -1;
+int path_shared = virStorageFileIsSharedFS(path);
uid_t uid = getuid();
gid_t gid = getgid();
@@ -2437,7 +2438,12 @@ qemuOpenFile(struct qemud_driver *driver, const char
*path, int oflags,
* in the failure case */
if (oflags O_CREAT) {
need_unlink = true;
-vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
+
+/* Don't force chown on network-shared FS
+ * as it is likely to fail. */
+if (path_shared = 0 || driver-dynamicOwnership)
+vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
+
if (stat(path, sb) == 0) {
is_reg = !!S_ISREG(sb.st_mode);
/* If the path is regular file which exists
@@ -2475,7 +2481,7 @@ qemuOpenFile(struct qemud_driver *driver, const char
*path, int oflags,
}
/* On Linux we can also verify the FS-type of the directory. */
-switch (virStorageFileIsSharedFS(path)) {
+switch (path_shared) {
case 1:
/* it was on a network share, so we'll continue
* as outlined above
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_agent: Report error class at least
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.
Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error
After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': CommandNotFound
---
src/qemu/qemu_agent.c | 14 ++
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index b759b7f..decfd0e 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1035,19 +1035,17 @@ static const char *
qemuAgentStringifyError(virJSONValuePtr error)
{
const char *klass = virJSONValueObjectGetString(error, class);
-const char *detail = NULL;
+const char *detail = virJSONValueObjectGetString(error, desc);
/* The QMP 'desc' field is usually sufficient for our generic
- * error reporting needs.
+ * error reporting needs. However, older agents did not provide
+ * any 'desc'. Reporting 'class' is not perfect but better
+ * than bare 'unknown error'.
*/
-if (klass)
-detail = virJSONValueObjectGetString(error, desc);
-
-
-if (!detail)
+if (!detail !klass)
detail = unknown QEMU command error;
-return detail;
+return detail ? detail : klass;
}
static const char *
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh list hangs
On 12.04.2012 10:40, Qian Zhang wrote: Hi, I am using RHEL 6.2 64bit, and the libvirt shipped in it is libvirt-0.9.4-23.el6.x86_64. I found sometimes the command virsh list hangs forever, and same issue for virt-manager which is always in Connecting status. But after restarting the libvirtd service, this issue is gone. Is this a bug of libvirt? Any help will be appreciated. I think I saw a bug in recent history about this, but now I can't find it. Anyway, please file a bug: https://bugzilla.redhat.com/ And attach debug logs. You can obtain them by editing /etc/libvirt/libvirtd.conf and setting: log_level=1 log_filters= log_outputs=1:file:/var/log/libvirtd.log For more information follow up http://libvirt.org/logging.html; For client log (we refer to virsh, virt-manager, ... as clients) you can set environment variables: LIBVIRT_DEBUG=1 LIBVIRT_LOG_OUTPUTS=1:file:client.log virt-manager --no-fork Then attach /var/log/libvirtd.log and client.log to the bugzilla. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuOpenFile: Don't force chown on NFS
On 12.04.2012 13:16, Laine Stump wrote: On 04/11/2012 04:21 PM, Laine Stump wrote: ACK to the idea, but NACK to the exact placement of the fix. On further examination (and actually doing a couple tests), I withdraw my NACK on the placement. I had mixed up usage of qemuOpenFile and virFileOpen in my memory - qemuOpenFile already hardly ever cares about the ownership of a file it opens, and even when it does, it always wants it to be owned by root (well, actually getuid()). So, although putting the fix in the place I suggested does work, my reasoning was flawed and your original position also works properly, as well as keeping the logic for setting the FORCE_OWNER flag in one place - ACK. Sorry for the noise :-) Not at all. In fact, I find this useful as somebody else has tested it. Pushed. Thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_agent: Report error class at least
On 12.04.2012 14:14, Daniel P. Berrange wrote:
On Thu, Apr 12, 2012 at 02:06:21PM +0200, Michal Privoznik wrote:
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.
Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error
After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': CommandNotFound
---
src/qemu/qemu_agent.c | 14 ++
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index b759b7f..decfd0e 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1035,19 +1035,17 @@ static const char *
qemuAgentStringifyError(virJSONValuePtr error)
{
const char *klass = virJSONValueObjectGetString(error, class);
-const char *detail = NULL;
+const char *detail = virJSONValueObjectGetString(error, desc);
/* The QMP 'desc' field is usually sufficient for our generic
- * error reporting needs.
+ * error reporting needs. However, older agents did not provide
+ * any 'desc'. Reporting 'class' is not perfect but better
+ * than bare 'unknown error'.
*/
-if (klass)
-detail = virJSONValueObjectGetString(error, desc);
-
-
-if (!detail)
+if (!detail !klass)
detail = unknown QEMU command error;
-return detail;
+return detail ? detail : klass;
}
You can get a list of all 'class' names that QEMU currently supports
from qerror.h. As we do for VIR_ERR_X constants, you should create
a mapping from QEMU class names, to error message strings.
Even better, share this mapping between the agent json monitor code
so we can improve error messages in both cases.
Regards,
Daniel
I don't think this should be shared; as written in commit message,
in case of json monitor we get 'desc' which fulfills translation
from 'class' to error message. Moreover, the 'desc' field already
contains correct values, e.g.:
#define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format':
%s, 'feature': %s } }
{execute:unknown command}
{error: {class: CommandNotFound, desc: The command unknown command has
not been found, data: {name: unknown command}}}
However, things change rapidly with GA:
{execute:unknown command}
{error: {class: CommandNotFound, data: {name: unknown command}}}
Therefore I see point in having translation table just for GA.
In fact, such table doesn't need to cover all error codes from qerror.h
only those used by GA:
~/work/qemu.git $ grep -r -o -e QERR_[A-Z_]* qga/ qapi* | cut -d':' -f 2 | sort
| uniq
QERR_BUFFER_OVERRUN
QERR_COMMAND_DISABLED
QERR_COMMAND_NOT_FOUND
QERR_FD_NOT_FOUND
QERR_INVALID_PARAMETER
QERR_INVALID_PARAMETER_TYPE
QERR_INVALID_PARAMETER_VALUE
QERR_OPEN_FILE_FAILED
QERR_QGA_COMMAND_FAILED
QERR_QGA_LOGGING_DISABLED
QERR_QMP_BAD_INPUT_OBJECT
QERR_QMP_BAD_INPUT_OBJECT_MEMBER
QERR_QMP_EXTRA_MEMBER
QERR_UNDEFINED_ERROR
QERR_UNSUPPORTED
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu_agent: Report error class at least
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.
Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error
After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': The command has not been found
---
src/qemu/qemu_agent.c | 37 +++--
1 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index b759b7f..bc4ceff 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1025,6 +1025,40 @@ cleanup:
return ret;
}
+static const char *
+qemuAgentStringifyErrorClass(const char *klass)
+{
+if (STREQ_NULLABLE(klass, BufferOverrun))
+return Buffer overrun;
+else if (STREQ_NULLABLE(klass, CommandDisabled))
+return The command has been disabled for this instance;
+else if (STREQ_NULLABLE(klass, CommandNotFound))
+return The command has not been found;
+else if (STREQ_NULLABLE(klass, FdNotFound))
+return File descriptor not found;
+else if (STREQ_NULLABLE(klass, InvalidParameter))
+return Invalid parameter;
+else if (STREQ_NULLABLE(klass, InvalidParameterType))
+return Invalid parameter type;
+else if (STREQ_NULLABLE(klass, InvalidParameterValue))
+return Invalid parameter value;
+else if (STREQ_NULLABLE(klass, OpenFileFailed))
+return Cannot open file;
+else if (STREQ_NULLABLE(klass, QgaCommandFailed))
+return Guest agent command failed;
+else if (STREQ_NULLABLE(klass, QMPBadInputObjectMember))
+return Bad QMP input object member;
+else if (STREQ_NULLABLE(klass, QMPExtraInputObjectMember))
+return Unexpected extra object member;
+else if (STREQ_NULLABLE(klass, UndefinedError))
+return An undefined error has occurred;
+else if (STREQ_NULLABLE(klass, Unsupported))
+return this feature or command is not currently supported;
+else if (klass)
+return klass;
+else
+return unknown QEMU command error;
+}
/* Ignoring OOM in this method, since we're already reporting
* a more important error
@@ -1043,9 +1077,8 @@ qemuAgentStringifyError(virJSONValuePtr error)
if (klass)
detail = virJSONValueObjectGetString(error, desc);
-
if (!detail)
-detail = unknown QEMU command error;
+detail = qemuAgentStringifyErrorClass(klass);
return detail;
}
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] blockjob: add virsh blockjob --wait
On 12.04.2012 21:59, Eric Blake wrote:
I'm tired of shell-scripting to wait for completion of a block pull,
when virsh can be taught to do the same. I couldn't quite reuse
vshWatchJob, as this is not a case of a long-running command where
a second thread must be used to probe job status (at least, not unless
I make virsh start doing blocking waits for an event to fire), but it
served as inspiration for my simpler single-threaded loop. There is
up to a half-second delay between sending SIGINT and the job being
aborted, but I didn't think it worth the complexity of a second thread
and use of poll() just to minimize that delay.
* tools/virsh.c (cmdBlockPull): Add new options to wait for
completion.
(blockJobImpl): Add argument.
(cmdBlockJob): Adjust caller.
* tools/virsh.pod (blockjob): Document new mode.
---
tools/virsh.c | 122 +-
tools/virsh.pod | 11 -
2 files changed, 119 insertions(+), 14 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8ef57e0..c54add9 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7274,11 +7274,11 @@ repoll:
if (pollfd.revents POLLIN
saferead(pipe_fd, retchar, sizeof(retchar)) 0
retchar == '0') {
-if (verbose) {
-/* print [100 %] */
-print_job_progress(label, 0, 1);
-}
-break;
+if (verbose) {
+/* print [100 %] */
+print_job_progress(label, 0, 1);
+}
+break;
}
goto cleanup;
}
@@ -7295,8 +7295,9 @@ repoll:
}
GETTIMEOFDAY(curr);
-if ( timeout ((int)(curr.tv_sec - start.tv_sec) * 1000 + \
- (int)(curr.tv_usec - start.tv_usec) / 1000)
timeout * 1000 ) {
+if (timeout (((int)(curr.tv_sec - start.tv_sec) * 1000 +
+ (int)(curr.tv_usec - start.tv_usec) / 1000)
+timeout * 1000)) {
/* suspend the domain when migration timeouts. */
vshDebug(ctl, VSH_ERR_DEBUG, %s timeout, label);
if (timeout_func)
These two chunks are rather cosmetic but I'm okay with having them in
this patch not a separate one.
@@ -7519,7 +7520,8 @@ typedef enum {
static int
blockJobImpl(vshControl *ctl, const vshCmd *cmd,
- virDomainBlockJobInfoPtr info, int mode)
+ virDomainBlockJobInfoPtr info, int mode,
+ virDomainPtr *pdom)
{
virDomainPtr dom = NULL;
const char *name, *path;
@@ -7560,7 +7562,9 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
}
cleanup:
-if (dom)
+if (pdom ret == 0)
+*pdom = dom;
+else if (dom)
virDomainFree(dom);
return ret;
}
@@ -7580,15 +7584,109 @@ static const vshCmdOptDef opts_block_pull[] = {
{bandwidth, VSH_OT_DATA, VSH_OFLAG_NONE, N_(Bandwidth limit in
MB/s)},
{base, VSH_OT_DATA, VSH_OFLAG_NONE,
N_(path of backing file in chain for a partial pull)},
+{wait, VSH_OT_BOOL, 0, N_(wait for job to finish)},
+{verbose, VSH_OT_BOOL, 0, N_(with --wait, display the progress)},
+{timeout, VSH_OT_INT, VSH_OFLAG_NONE,
+ N_(with --wait, abort if pull exceeds timeout (in seconds))},
{NULL, 0, 0, NULL}
};
static bool
cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
{
-if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_PULL) != 0)
+virDomainPtr dom = NULL;
+bool ret = false;
+bool blocking = vshCommandOptBool(cmd, wait);
+bool verbose = vshCommandOptBool(cmd, verbose);
+int timeout = 0;
+struct sigaction sig_action;
+struct sigaction old_sig_action;
+sigset_t sigmask;
+struct timeval start;
+struct timeval curr;
+const char *path = NULL;
+bool quit = false;
+
+if (blocking) {
+if (vshCommandOptInt(cmd, timeout, timeout) 0) {
+if (timeout 1) {
+vshError(ctl, %s, _(migrate: Invalid timeout));
+return false;
+}
+
+/* Ensure that we can multiply by 1000 without overflowing. */
+if (timeout INT_MAX / 1000) {
+vshError(ctl, %s, _(migrate: Timeout is too big));
+return false;
+}
+}
+if (vshCommandOptString(cmd, path, path) 0)
+return false;
+
+sigemptyset(sigmask);
+sigaddset(sigmask, SIGINT);
+
+intCaught = 0;
+sig_action.sa_sigaction = vshCatchInt;
+sigemptyset(sig_action.sa_mask);
+sigaction(SIGINT, sig_action, old_sig_action);
+
+GETTIMEOFDAY(start);
+} else if (verbose || vshCommandOptBool(cmd, timeout)) {
+vshError(ctl, %s, _(blocking control options require
[libvirt] [PATCH 0/3] Couple of cleanups in qemu code
Michal Privoznik (3): qemu: Fix mem leak in qemuProcessInitCpuAffinity gitignore: Reorder alphabetically qemu: Split if condition in qemuDomainSnapshotUndoSingleDiskActive .gitignore |6 +++--- src/qemu/qemu_driver.c |9 +++-- src/qemu/qemu_process.c | 23 +-- 3 files changed, 23 insertions(+), 15 deletions(-) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] gitignore: Reorder alphabetically
Recent git reorders .gitignore alphabetically. However, changes are not committed and I am tired of discarding these changes from my patches. --- .gitignore |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 5aa9c9b..14a21d0 100644 --- a/.gitignore +++ b/.gitignore @@ -48,12 +48,12 @@ /daemon/*_dispatch.h /daemon/libvirt_qemud /daemon/libvirtd -/daemon/libvirtd.init -/daemon/libvirtd.service /daemon/libvirtd*.logrotate /daemon/libvirtd.8 /daemon/libvirtd.8.in +/daemon/libvirtd.init /daemon/libvirtd.pod +/daemon/libvirtd.service /docs/devhelp/libvirt.devhelp /docs/hvsupport.html.in /docs/libvirt-api.xml @@ -118,6 +118,7 @@ /tests/eventtest /tests/hashtest /tests/jsontest +/tests/libvirtdconftest /tests/networkxml2argvtest /tests/nodeinfotest /tests/nwfilterxml2xmltest @@ -150,7 +151,6 @@ /tests/vmx2xmltest /tests/xencapstest /tests/xmconfigtest -/tests/libvirtdconftest /tools/*.[18] /tools/libvirt-guests.init /tools/virsh -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: Fix mem leak in qemuProcessInitCpuAffinity
If placement mode is AUTO, on some return paths char *cpumap or
char *nodeset are leaked.
---
src/qemu/qemu_process.c | 23 +--
1 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 19569cf..692fc32 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1796,6 +1796,7 @@ static int
qemuProcessInitCpuAffinity(struct qemud_driver *driver,
virDomainObjPtr vm)
{
+int ret = -1;
int i, hostcpus, maxcpu = QEMUD_CPUMASK_LEN;
virNodeInfo nodeinfo;
unsigned char *cpumap;
@@ -1824,19 +1825,21 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver,
nodeset = qemuGetNumadAdvice(vm-def);
if (!nodeset)
-return -1;
+goto cleanup;
if (VIR_ALLOC_N(tmp_cpumask, VIR_DOMAIN_CPUMASK_LEN) 0) {
virReportOOMError();
-return -1;
+VIR_FREE(nodeset);
+goto cleanup;
}
if (virDomainCpuSetParse(nodeset, 0, tmp_cpumask,
VIR_DOMAIN_CPUMASK_LEN) 0) {
VIR_FREE(tmp_cpumask);
VIR_FREE(nodeset);
-return -1;
+goto cleanup;
}
+VIR_FREE(nodeset);
for (i = 0; i maxcpu i VIR_DOMAIN_CPUMASK_LEN; i++) {
if (tmp_cpumask[i])
@@ -1849,7 +1852,6 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver,
VIR_WARN(Unable to save status on vm %s after state change,
vm-def-name);
}
-VIR_FREE(nodeset);
} else {
if (vm-def-cpumask) {
/* XXX why don't we keep 'cpumask' in the libvirt cpumap
@@ -1872,13 +1874,14 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver,
* running at this point
*/
if (virProcessInfoSetAffinity(0, /* Self */
- cpumap, cpumaplen, maxcpu) 0) {
-VIR_FREE(cpumap);
-return -1;
-}
+ cpumap, cpumaplen, maxcpu) 0)
+goto cleanup;
+
+ret = 0;
+
+cleanup:
VIR_FREE(cpumap);
-
-return 0;
+return ret;
}
/* set link states to down on interfaces at qemu start */
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: Split if condition in qemuDomainSnapshotUndoSingleDiskActive
Since compilers are trying to optimize code they are allowed to reorder evaluation of conditions in if statement (okay, not in all cases, but they can in this one). Therefore if we do: if (stat(file, st) == 0 unlink(file) 0) after compiler chews this it may get feeling that swapping order is a good idea. However, we obviously don't want to call stat() on just unlink()-ed file. --- src/qemu/qemu_driver.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 65ed290..037d45c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9998,9 +9998,14 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver, VIR_WARN(Unable to restore security label on %s, disk-src); if (virDomainLockDiskDetach(driver-lockManager, vm, disk) 0) VIR_WARN(Unable to release lock on %s, disk-src); + +/* Deliberately do not join these two ifs. Compiler may mix up + * the order of evaluation so unlink() may proceed stat() + * which is not what we want */ if (need_unlink stat(disk-src, st) == 0 -st.st_size == 0 S_ISREG(st.st_mode) unlink(disk-src) 0) -VIR_WARN(Unable to remove just-created %s, disk-src); +st.st_size == 0 S_ISREG(st.st_mode)) +if (unlink(disk-src) 0) +VIR_WARN(Unable to remove just-created %s, disk-src); /* Update vm in place to match changes. */ VIR_FREE(disk-src); -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/3] conf: Avoid double assignment in virDomainDiskRemove
Although it should be harmless to do:
disk = disk = def-disks[i]
some not-so-wise compilers may fool around.
Besides, such assignment is useless here.
---
src/conf/domain_conf.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c6b97e1..a9c5cbc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7124,7 +7124,7 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def,
virDomainDiskDefPtr
virDomainDiskRemove(virDomainDefPtr def, size_t i)
{
-virDomainDiskDefPtr disk = disk = def-disks[i];
+virDomainDiskDefPtr disk = def-disks[i];
if (def-ndisks 1) {
memmove(def-disks + i,
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: Fix mem leak in qemuProcessInitCpuAffinity
On 13.04.2012 12:24, Osier Yang wrote: On 04/13/2012 05:12 PM, Michal Privoznik wrote: If placement mode is AUTO, on some return paths char *cpumap or char *nodeset are leaked. --- src/qemu/qemu_process.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) ACK. Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/3] conf: Avoid double assignment in virDomainDiskRemove
On 13.04.2012 12:25, Osier Yang wrote: On 04/13/2012 05:24 PM, Michal Privoznik wrote: Although it should be harmless to do: disk = disk = def-disks[i] some not-so-wise compilers may fool around. Besides, such assignment is useless here. --- src/conf/domain_conf.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] gitignore: Reorder alphabetically
On 13.04.2012 14:28, Osier Yang wrote: On 04/13/2012 05:12 PM, Michal Privoznik wrote: Recent git reorders .gitignore alphabetically. However, changes are not committed and I am tired of discarding these changes from my patches. --- .gitignore |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) ACK Osier Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: fix path to openvz network configuration file
On 16.04.2012 16:21, Guido Günther wrote: It's vznet.conf not vznetctl.conf, see e.g.: http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetcfg.in;h=e91f5c4a0744c1ea149e1b8c241b666052e10b12;hb=HEAD --- docs/drvopenvz.html.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Qualifies as trivial. Anyway, ACK. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RESEND] qemuProcessStart: Switch to flags instead of bunch booleans
Currently, we have 3 boolean arguments we have to pass
to qemuProcessStart(). As libvirt grows it is harder and harder
to remember them and their position. Therefore we should
switch to flags instead.
---
This is just rebased version of:
http://www.redhat.com/archives/libvir-list/2012-March/msg00331.html
src/qemu/qemu_driver.c| 45 +
src/qemu/qemu_migration.c |7 ---
src/qemu/qemu_process.c | 21 +
src/qemu/qemu_process.h | 12
4 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 65ed290..436ef37 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1351,10 +1351,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
virDomainPtr dom = NULL;
virDomainEventPtr event = NULL;
virDomainEventPtr event2 = NULL;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
virCheckFlags(VIR_DOMAIN_START_PAUSED |
VIR_DOMAIN_START_AUTODESTROY, NULL);
+if (flags VIR_DOMAIN_START_PAUSED)
+start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
+if (flags VIR_DOMAIN_START_AUTODESTROY)
+start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY;
+
qemuDriverLock(driver);
if (!(def = virDomainDefParseString(driver-caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
@@ -1383,10 +1389,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup; /* free the 'vm' we created ? */
-if (qemuProcessStart(conn, driver, vm, NULL, true,
- (flags VIR_DOMAIN_START_PAUSED) != 0,
- (flags VIR_DOMAIN_START_AUTODESTROY) != 0,
- -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE)
0) {
+if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ start_flags) 0) {
virDomainAuditStart(vm, booted, false);
if (qemuDomainObjEndJob(driver, vm) 0)
qemuDomainRemoveInactive(driver, vm);
@@ -4138,9 +4143,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
}
/* Set the migration source and start it up. */
-ret = qemuProcessStart(conn, driver, vm, stdio, false, true,
- false, *fd, path, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
+ret = qemuProcessStart(conn, driver, vm, stdio, *fd, path, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
+ VIR_QEMU_PROCESS_START_PAUSED);
if (intermediatefd != -1) {
if (ret 0) {
@@ -4710,6 +4715,10 @@ qemuDomainObjStart(virConnectPtr conn,
bool autodestroy = (flags VIR_DOMAIN_START_AUTODESTROY) != 0;
bool bypass_cache = (flags VIR_DOMAIN_START_BYPASS_CACHE) != 0;
bool force_boot = (flags VIR_DOMAIN_START_FORCE_BOOT) != 0;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
+
+start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
+start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0;
/*
* If there is a managed saved state restore it instead of starting
@@ -4741,9 +4750,8 @@ qemuDomainObjStart(virConnectPtr conn,
}
}
-ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused,
- autodestroy, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags);
virDomainAuditStart(vm, booted, ret = 0);
if (ret = 0) {
virDomainEventPtr event =
@@ -11027,9 +11035,10 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (config)
virDomainObjAssignDef(vm, config, false);
-rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- false, true, false, -1, NULL, snap,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+rc = qemuProcessStart(snapshot-domain-conn,
+ driver, vm, NULL, -1, NULL, snap,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED);
virDomainAuditStart(vm, from-snapshot, rc = 0);
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
event = virDomainEventNewFromObj(vm,
@@ -4,12 +11123,16 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
/* Flush first event, now do transition 2 or 3
[libvirt] [PATCH v2] qemuProcessStart: Switch to flags instead of bunch booleans
Currently, we have 3 boolean arguments we have to pass
to qemuProcessStart(). As libvirt grows it is harder and harder
to remember them and their position. Therefore we should
switch to flags instead.
---
diff to v1:
-fix a test for START_PAUSED flag
src/qemu/qemu_driver.c| 45 +
src/qemu/qemu_migration.c |7 ---
src/qemu/qemu_process.c | 21 +
src/qemu/qemu_process.h | 12
4 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 65ed290..436ef37 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1351,10 +1351,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
virDomainPtr dom = NULL;
virDomainEventPtr event = NULL;
virDomainEventPtr event2 = NULL;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
virCheckFlags(VIR_DOMAIN_START_PAUSED |
VIR_DOMAIN_START_AUTODESTROY, NULL);
+if (flags VIR_DOMAIN_START_PAUSED)
+start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
+if (flags VIR_DOMAIN_START_AUTODESTROY)
+start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY;
+
qemuDriverLock(driver);
if (!(def = virDomainDefParseString(driver-caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
@@ -1383,10 +1389,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr
conn, const char *xml,
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup; /* free the 'vm' we created ? */
-if (qemuProcessStart(conn, driver, vm, NULL, true,
- (flags VIR_DOMAIN_START_PAUSED) != 0,
- (flags VIR_DOMAIN_START_AUTODESTROY) != 0,
- -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE)
0) {
+if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ start_flags) 0) {
virDomainAuditStart(vm, booted, false);
if (qemuDomainObjEndJob(driver, vm) 0)
qemuDomainRemoveInactive(driver, vm);
@@ -4138,9 +4143,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
}
/* Set the migration source and start it up. */
-ret = qemuProcessStart(conn, driver, vm, stdio, false, true,
- false, *fd, path, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
+ret = qemuProcessStart(conn, driver, vm, stdio, *fd, path, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
+ VIR_QEMU_PROCESS_START_PAUSED);
if (intermediatefd != -1) {
if (ret 0) {
@@ -4710,6 +4715,10 @@ qemuDomainObjStart(virConnectPtr conn,
bool autodestroy = (flags VIR_DOMAIN_START_AUTODESTROY) != 0;
bool bypass_cache = (flags VIR_DOMAIN_START_BYPASS_CACHE) != 0;
bool force_boot = (flags VIR_DOMAIN_START_FORCE_BOOT) != 0;
+unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
+
+start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
+start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0;
/*
* If there is a managed saved state restore it instead of starting
@@ -4741,9 +4750,8 @@ qemuDomainObjStart(virConnectPtr conn,
}
}
-ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused,
- autodestroy, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags);
virDomainAuditStart(vm, booted, ret = 0);
if (ret = 0) {
virDomainEventPtr event =
@@ -11027,9 +11035,10 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (config)
virDomainObjAssignDef(vm, config, false);
-rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- false, true, false, -1, NULL, snap,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+rc = qemuProcessStart(snapshot-domain-conn,
+ driver, vm, NULL, -1, NULL, snap,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED);
virDomainAuditStart(vm, from-snapshot, rc = 0);
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
event = virDomainEventNewFromObj(vm,
@@ -4,12 +11123,16 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
/* Flush first event, now do transition 2 or 3 */
bool paused = (flags
Re: [libvirt] [PATCH v2] qemuProcessStart: Switch to flags instead of bunch booleans
On 16.04.2012 17:26, Daniel P. Berrange wrote: On Mon, Apr 16, 2012 at 05:09:59PM +0200, Michal Privoznik wrote: Currently, we have 3 boolean arguments we have to pass to qemuProcessStart(). As libvirt grows it is harder and harder to remember them and their position. Therefore we should switch to flags instead. --- diff to v1: -fix a test for START_PAUSED flag src/qemu/qemu_driver.c| 45 + src/qemu/qemu_migration.c |7 --- src/qemu/qemu_process.c | 21 + src/qemu/qemu_process.h | 12 4 files changed, 54 insertions(+), 31 deletions(-) ACK, that's better now Daniel Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks
On 19.04.2012 08:25, Chen Hanxiao wrote: From: Chen Hanxiao chenhanx...@cn.fujitsu.com When adding a large number of virtio storage devices to virtual machine by using 'virsh edit' command, there is a problem: When we added more than 190 virtio disks by 'virsh edit' command, we got a feedback as 'error: Unable to encode message payload'. In virt-mananger, the same defect occurs with about 180 virtio disks added. PCI devices are limited by the virtualized system architecture. Out of the 32 available PCI devices for a guest, 4 are not removable. This means there are up to 28 free PCI slots available for additional devices per guest. Each PCI device in a guest can have up to 8 functions. One slot kept for network interface, we could add at most 216 (27*8) virtio disks. The length of xml description with multifuction for one virtio disk is about 290 characters; In virt-manger, an extra tag 'alias name' will come to the xml description, which would add about 40 more characters. So for one xml description, 330 characters would be enough. A brand new virtual machine with one IDE bus disk needs about 1900 characters for xml description. In src/remote/remote_protocol.x, there is a limitation for XDR enoding length as REMOTE_STRING_MAX. According to the analysis above, at least 73180(1900+216*330) characters are needed for 216 virtio disks. In the view of variable length in tag 'source file', so I think it would be better to extend this limitation from 65536 to 8. --- src/remote/remote_protocol.x |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2d57247..2c8dcbb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -65,7 +65,7 @@ * This is an arbitrary limit designed to stop the decoder from trying * to allocate unbounded amounts of memory when fed with a bad message. */ -const REMOTE_STRING_MAX = 65536; +const REMOTE_STRING_MAX = 8; /* A long string, which may NOT be NULL. */ typedef string remote_nonnull_stringREMOTE_STRING_MAX; While this limit is pretty harmless, the much bigger issue is - a per message buffer of VIR_NET_MESSAGE_MAX bytes which is hold during whole API. I am writing a code to allocate this buffer dynamically based on message length. btw: don't you want size up VIR_NET_MESSAGE_STRING_MAX in virnetprotocol.x as well? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks
On 19.04.2012 14:45, Richard W.M. Jones wrote: On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote: * This is an arbitrary limit designed to stop the decoder from trying * to allocate unbounded amounts of memory when fed with a bad message. */ -const REMOTE_STRING_MAX = 65536; +const REMOTE_STRING_MAX = 8; Can this limit be changed? I thought it would break existing clients or servers. Rich. Yes no. There are 4 possibilities: - old both server client: long string get dropped at server side and unable to encode string/payload error is thrown. - old server new client: if server wants to send long string it's the previous case; client will successfully retransmit message to the daemon where it gets later dropped because of limit violation. - new server old client: see previous case - new server new client: nothing gets broken, everything works. The only problem is with these gimme-list-of-* APIs. Because here you'll get only partial result (first N entries - based on RPC limit). Thus, changing this limit is okay. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib][PATCH] test-domain-create: Don't shadow global variable
In function create_usb_controller variable 'index' shadows a global
declaration.
---
libvirt-gconfig/tests/test-domain-create.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt-gconfig/tests/test-domain-create.c
b/libvirt-gconfig/tests/test-domain-create.c
index 699c255..00bd620 100644
--- a/libvirt-gconfig/tests/test-domain-create.c
+++ b/libvirt-gconfig/tests/test-domain-create.c
@@ -45,7 +45,7 @@ const char *features[] = { foo, bar, baz, NULL };
static GVirConfigDomainControllerUsb *
-create_usb_controller(GVirConfigDomainControllerUsbModel model, guint index,
+create_usb_controller(GVirConfigDomainControllerUsbModel model, guint indx,
GVirConfigDomainControllerUsb *master, guint start_port,
guint domain, guint bus, guint slot, guint function,
gboolean multifunction)
@@ -55,7 +55,7 @@ create_usb_controller(GVirConfigDomainControllerUsbModel
model, guint index,
controller = gvir_config_domain_controller_usb_new();
gvir_config_domain_controller_usb_set_model(controller, model);
-
gvir_config_domain_controller_set_index(GVIR_CONFIG_DOMAIN_CONTROLLER(controller),
index);
+
gvir_config_domain_controller_set_index(GVIR_CONFIG_DOMAIN_CONTROLLER(controller),
indx);
if (master)
gvir_config_domain_controller_usb_set_master(controller, master,
start_port);
address = gvir_config_domain_address_pci_new();
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib][PATCH] test-domain-create: Don't shadow global variable
On 24.04.2012 14:48, Daniel P. Berrange wrote: On Tue, Apr 24, 2012 at 02:03:15PM +0200, Michal Privoznik wrote: In function create_usb_controller variable 'index' shadows a global declaration. --- libvirt-gconfig/tests/test-domain-create.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK Daniel Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: Update libvirt-daemon-arch figure
Currently, we are showing libvirt client contains qemu and lxc drivers. This is not correct as qemu and lxc drivers are always accessed via remote driver. Therefore we need to substitute those two so we don't confuse users. I've chosen vbox and vmware. --- docs/libvirt-daemon-arch.fig | 62 +- docs/libvirt-daemon-arch.png | Bin 16479 - 8070 bytes 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/docs/libvirt-daemon-arch.fig b/docs/libvirt-daemon-arch.fig index 53e9c6d..d352583 100644 --- a/docs/libvirt-daemon-arch.fig +++ b/docs/libvirt-daemon-arch.fig @@ -1,72 +1,72 @@ -#FIG 3.2 Produced by xfig version 3.2.5 +#FIG 3.2 Produced by xfig version 3.2.5b Landscape Center Inches -Letter +Letter 100.00 Single -2 1200 2 -6 75 150 13050 5100 +6 75 150 13050 5025 6 4425 764 5938 1150 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 4425 764 5938 764 5938 1150 4425 1150 4425 764 -4 0 0 50 -1 16 15 0. 4 131 415 4519 1053 xen\001 +4 0 0 50 -1 16 15 0. 4 135 390 4519 1053 xen\001 -6 6 4425 1246 5938 1631 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 4425 1246 5938 1246 5938 1631 4425 1631 4425 1246 -4 0 0 50 -1 16 15 0. 4 178 640 4519 1535 qemu\001 +4 0 0 50 -1 16 15 0. 4 180 510 4519 1535 vbox\001 -6 6 4425 1728 5938 2113 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 4425 1728 5938 1728 5938 2113 4425 2113 4425 1728 -4 0 0 50 -1 16 15 0. 4 178 829 4519 2017 openvz\001 +4 0 0 50 -1 16 15 0. 4 195 780 4519 2017 openvz\001 -6 6 4425 2210 5938 2595 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 4425 2210 5938 2210 5938 2595 4425 2595 4425 2210 -4 0 0 50 -1 16 15 0. 4 178 320 4519 2499 lxc\001 +4 0 0 50 -1 16 15 0. 4 135 855 4519 2499 vmware\001 -6 6 4425 2691 5938 3077 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 4425 2691 5938 2691 5938 3077 4425 3077 4425 2691 -4 0 0 50 -1 16 15 0. 4 166 415 4519 2980 test\001 --6 -6 4425 3173 5938 3559 -2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 -4425 3173 5938 3173 5938 3559 4425 3559 4425 3173 -4 0 0 50 -1 16 15 0. 4 166 794 4519 3462 remote\001 +4 0 0 50 -1 16 15 0. 4 165 405 4519 2980 test\001 -6 6 11328 764 12842 1150 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 11328 764 12842 764 12842 1150 11328 1150 11328 764 -4 0 0 50 -1 16 15 0. 4 131 415 11423 1053 xen\001 +4 0 0 50 -1 16 15 0. 4 135 390 11423 1053 xen\001 -6 6 11328 1246 12842 1631 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 11328 1246 12842 1246 12842 1631 11328 1631 11328 1246 -4 0 0 50 -1 16 15 0. 4 178 640 11423 1535 qemu\001 +4 0 0 50 -1 16 15 0. 4 195 615 11423 1535 qemu\001 -6 6 11328 1728 12842 2113 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 11328 1728 12842 1728 12842 2113 11328 2113 11328 1728 -4 0 0 50 -1 16 15 0. 4 178 829 11423 2017 openvz\001 +4 0 0 50 -1 16 15 0. 4 195 780 11423 2017 openvz\001 -6 6 11328 2210 12842 2595 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 11328 2210 12842 2210 12842 2595 11328 2595 11328 2210 -4 0 0 50 -1 16 15 0. 4 178 320 11423 2499 lxc\001 +4 0 0 50 -1 16 15 0. 4 180 285 11423 2499 lxc\001 -6 6 11328 2691 12842 3077 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 11328 2691 12842 2691 12842 3077 11328 3077 11328 2691 -4 0 0 50 -1 16 15 0. 4 166 415 11423 2980 test\001 +4 0 0 50 -1 16 15 0. 4 165 405 11423 2980 test\001 -6 6 11328 3173 12842 3559 2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 11328 3173 12842 3173 12842 3559 11328 3559 11328 3173 -4 0 0 50 -1 16 15 0. 4 166 794 11423 3462 remote\001 +4 0 0 50 -1 16 15 0. 4 165 765 11423 3462 remote\001 +-6 +6 4425 3173 5938 3559 +2 2 0 1 0 7 50 -1 -1 4.000 0 0 7 0 0 5 +4425 3173 5938 3173 5938 3559 4425 3559 4425 3173 +4 0 0 50 -1 16 15 0. 4 165 765 4519 3462 remote\001 -6 2 2 0 1 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 75 1342 1587 1342 1587 2113 75 2113 75 1342 @@ -99,16 +99,16 @@ Single 10004 572 10004 3751 2 1 2 5 0 7 50 -1 -1 3.000 0 0 -1 0 0 2 10760 572 10760 3751 -4 0 0 50 -1 16 15 0. 4 225 1256 170 1728 Application\001 -4 0 0 50 -1 16 13 0. 4 154 367 1966 1631 URI\001 -4 0 0 50 -1 16 15 5.3233 4 178 1149 3763 4040 Driver API\001 -4 0 0 50 -1 16 15 5.3233 4 178 1173 3101 4040 Public API\001 -4 0 0 50 -1 16 15 5.3233 4 225 1233 4992 4040 Driver Impl\001 -4 0 0 50 -1 16 18 0. 4 213 652 3763 379 libvirt\001 -4 0 0 50 -1 16 13 0. 4 154 901 1966 2017 lxc://host/\001 -4 0 0 50 -1 16 15 0. 4 178 747 7734 1728 libvirtd\001 -4 0 0 50 -1 16 15 5.3233 4 178 1149 10666 4040 Driver API\001 -4 0 0 50 -1 16 15 5.3233 4 178 1173 10004 4040 Public API\001 -4 0 0 50 -1 16 15 5.3233 4 225 1233 11896 4040 Driver Impl\001 -4 0 0 50 -1 16 18 0. 4 213 652 10666 379 libvirt\001 +4 0 0 50 -1 16 15 0. 4 240 1170 170 1728 Application\001 +4 0 0 50 -1 16 13 0. 4 165 360 1966 1631 URI\001 +4 0 0 50 -1 16 15 5.3233 4 180 1095 3763 4040 Driver API\001 +4 0 0 50 -1 16
[libvirt] [PATCH] virNetDevMacVLanVPortProfileRegisterCallback: Fix segfault
Currently, we are calling memcpy(virtPortProfile, ...) unconditionally.
Which means if virtPortProfile is NULL we SIGSEGV. Therefore, add check
to call memcpy() conditionally.
(gdb) bt
#0 virNetDevMacVLanVPortProfileRegisterCallback (ifname=0x73f1ee60
macvtap0, macaddress=0x7fffe8096e34 RT, linkdev=0x7fffe8098b90 eth0,
vmuuid=0x7fffe8099558
l\220*\237u`\326\213\325\227F\f-B\bD0g\t\350\377\177, virtPortProfile=0x0,
vmOp=VIR_NETDEV_VPORT_PROFILE_OP_CREATE) at /usr/include/bits/string3.h:52
#1 0x77782446 in virNetDevMacVLanCreateWithVPortProfile
(tgifname=optimized out, macaddress=0x7fffe8096e34 RT,
linkdev=0x7fffe8098b90 eth0, mode=optimized out, withTap=true, vnet_hdr=0,
vmuuid=0x7fffe8099558
l\220*\237u`\326\213\325\227F\f-B\bD0g\t\350\377\177, virtPortProfile=0x0,
res_ifname=0x73f1ef00, vmOp=VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
stateDir=0x7fffe800d900 /var/run/libvirt/qemu, bandwidth=0x0) at
util/virnetdevmacvlan.c:954
#2 0x004738d4 in qemuPhysIfaceConnect ()
#3 0x0047e5f3 in qemuBuildCommandLine ()
#4 0x0049a404 in qemuProcessStart ()
#5 0x00468ac9 in qemuDomainObjStart ()
#6 0x00469112 in qemuDomainStartWithFlags ()
#7 0x777ef0c6 in virDomainCreate (domain=0x768cb0) at libvirt.c:8162
#8 0x0043e158 in remoteDispatchDomainCreateHelper ()
#9 0x7783d835 in virNetServerProgramDispatchCall (msg=0x7fffe80cbe50,
client=0x7699d0, server=0x7658a0, prog=0x770ab0) at
rpc/virnetserverprogram.c:416
#10 virNetServerProgramDispatch (prog=0x770ab0, server=0x7658a0,
client=0x7699d0, msg=0x7fffe80cbe50) at rpc/virnetserverprogram.c:289
#11 0x77839961 in virNetServerHandleJob (jobOpaque=optimized out,
opaque=0x7658a0) at rpc/virnetserver.c:161
#12 0x7776c1ce in virThreadPoolWorker (opaque=optimized out) at
util/threadpool.c:144
#13 0x7776b876 in virThreadHelper (data=optimized out) at
util/threads-pthread.c:161
#14 0x774f0e2c in start_thread () from /lib64/libpthread.so.0
#15 0x7723766d in clone () from /lib64/libc.so.6
---
src/util/virnetdevmacvlan.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 879d846..c2fd6d0 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -769,9 +769,11 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char
*ifname,
goto memory_error;
if ((calld-cr_ifname = strdup(ifname)) == NULL)
goto memory_error;
-if (VIR_ALLOC(calld-virtPortProfile) 0)
-goto memory_error;
-memcpy(calld-virtPortProfile, virtPortProfile,
sizeof(*virtPortProfile));
+if (virtPortProfile) {
+if (VIR_ALLOC(calld-virtPortProfile) 0)
+goto memory_error;
+memcpy(calld-virtPortProfile, virtPortProfile,
sizeof(*virtPortProfile));
+}
if (VIR_ALLOC_N(calld-macaddress, VIR_MAC_BUFLEN) 0)
goto memory_error;
memcpy(calld-macaddress, macaddress, VIR_MAC_BUFLEN);
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetDevMacVLanVPortProfileRegisterCallback: Fix segfault
On 24.04.2012 17:52, Michal Privoznik wrote:
Currently, we are calling memcpy(virtPortProfile, ...) unconditionally.
Which means if virtPortProfile is NULL we SIGSEGV. Therefore, add check
to call memcpy() conditionally.
(gdb) bt
#0 virNetDevMacVLanVPortProfileRegisterCallback (ifname=0x73f1ee60
macvtap0, macaddress=0x7fffe8096e34 RT, linkdev=0x7fffe8098b90 eth0,
vmuuid=0x7fffe8099558
l\220*\237u`\326\213\325\227F\f-B\bD0g\t\350\377\177, virtPortProfile=0x0,
vmOp=VIR_NETDEV_VPORT_PROFILE_OP_CREATE) at /usr/include/bits/string3.h:52
#1 0x77782446 in virNetDevMacVLanCreateWithVPortProfile
(tgifname=optimized out, macaddress=0x7fffe8096e34 RT,
linkdev=0x7fffe8098b90 eth0, mode=optimized out, withTap=true, vnet_hdr=0,
vmuuid=0x7fffe8099558
l\220*\237u`\326\213\325\227F\f-B\bD0g\t\350\377\177, virtPortProfile=0x0,
res_ifname=0x73f1ef00, vmOp=VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
stateDir=0x7fffe800d900 /var/run/libvirt/qemu, bandwidth=0x0) at
util/virnetdevmacvlan.c:954
#2 0x004738d4 in qemuPhysIfaceConnect ()
#3 0x0047e5f3 in qemuBuildCommandLine ()
#4 0x0049a404 in qemuProcessStart ()
#5 0x00468ac9 in qemuDomainObjStart ()
#6 0x00469112 in qemuDomainStartWithFlags ()
#7 0x777ef0c6 in virDomainCreate (domain=0x768cb0) at libvirt.c:8162
#8 0x0043e158 in remoteDispatchDomainCreateHelper ()
#9 0x7783d835 in virNetServerProgramDispatchCall
(msg=0x7fffe80cbe50, client=0x7699d0, server=0x7658a0, prog=0x770ab0) at
rpc/virnetserverprogram.c:416
#10 virNetServerProgramDispatch (prog=0x770ab0, server=0x7658a0,
client=0x7699d0, msg=0x7fffe80cbe50) at rpc/virnetserverprogram.c:289
#11 0x77839961 in virNetServerHandleJob (jobOpaque=optimized out,
opaque=0x7658a0) at rpc/virnetserver.c:161
#12 0x7776c1ce in virThreadPoolWorker (opaque=optimized out) at
util/threadpool.c:144
#13 0x7776b876 in virThreadHelper (data=optimized out) at
util/threads-pthread.c:161
#14 0x774f0e2c in start_thread () from /lib64/libpthread.so.0
#15 0x7723766d in clone () from /lib64/libc.so.6
---
src/util/virnetdevmacvlan.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 879d846..c2fd6d0 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -769,9 +769,11 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char
*ifname,
goto memory_error;
if ((calld-cr_ifname = strdup(ifname)) == NULL)
goto memory_error;
-if (VIR_ALLOC(calld-virtPortProfile) 0)
-goto memory_error;
-memcpy(calld-virtPortProfile, virtPortProfile,
sizeof(*virtPortProfile));
+if (virtPortProfile) {
+if (VIR_ALLOC(calld-virtPortProfile) 0)
+goto memory_error;
+memcpy(calld-virtPortProfile, virtPortProfile,
sizeof(*virtPortProfile));
+}
if (VIR_ALLOC_N(calld-macaddress, VIR_MAC_BUFLEN) 0)
goto memory_error;
memcpy(calld-macaddress, macaddress, VIR_MAC_BUFLEN);
Self NACK; It turned out to be race because it's still reproducible in
some cases: if(virPortProfile) evaluates to true; however gdb still
catches SIGSEGV in memcpy(). Meanwhile, something free() virPortProfile,
therefore gdb is unable to print any non-NULL value.
Maybe it's related to qemu process dying right after it was started.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
