[libvirt] [PATCH 0/3] fix some issue around host-passthough cpu model
We support add feature in host-passthough cpu model, but there are some place need fix. Luyao Huang (3): cpu:x86: fix cannot keep cpu feature after migrate/restore qemu: add a check for non-migratable cpu flags docs: fix a small xml error in docs docs/formatdomain.html.in | 2 +- src/cpu/cpu_x86.c | 9 - src/qemu/qemu_migration.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] cpu:x86: fix cannot keep cpu feature after migrate/restore
https://bugzilla.redhat.com/show_bug.cgi?id=1207095
When we set feature when cpu model is host-passthrough and start the vm,
we can see these settings in the XML, however after migrate/restore these
flags will be covered by host cpu models.
As cpu model host-passthrough can set feature by users now, so we need
check if there is a feature when do updatecpu().
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/cpu/cpu_x86.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index bf1867b..81ab2ca 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2087,8 +2087,7 @@ x86UpdateCustom(virCPUDefPtr guest,
static int
x86UpdateHostModel(virCPUDefPtr guest,
- const virCPUDef *host,
- bool passthrough)
+ const virCPUDef *host)
{
virCPUDefPtr oldguest = NULL;
const struct x86_map *map;
@@ -2118,7 +2117,7 @@ x86UpdateHostModel(virCPUDefPtr guest,
}
}
}
-for (i = 0; !passthrough i oldguest-nfeatures; i++) {
+for (i = 0; i oldguest-nfeatures; i++) {
if (virCPUDefUpdateFeature(guest,
oldguest-features[i].name,
oldguest-features[i].policy) 0)
@@ -2143,11 +2142,11 @@ x86Update(virCPUDefPtr guest,
case VIR_CPU_MODE_HOST_MODEL:
guest-match = VIR_CPU_MATCH_EXACT;
-return x86UpdateHostModel(guest, host, false);
+return x86UpdateHostModel(guest, host);
case VIR_CPU_MODE_HOST_PASSTHROUGH:
guest-match = VIR_CPU_MATCH_MINIMUM;
-return x86UpdateHostModel(guest, host, true);
+return x86UpdateHostModel(guest, host);
case VIR_CPU_MODE_LAST:
break;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Add uptime to API returning Dom Info
On Mon, Mar 30, 2015 at 13:26:40 +0530, Nehal J Wani wrote:
On Mon, Mar 30, 2015 at 1:16 PM, Peter Krempa pkre...@redhat.com wrote:
On Mon, Mar 30, 2015 at 03:29:40 +0530, Nehal J Wani wrote:
This is an attempt to fix:
https://bugzilla.redhat.com/show_bug.cgi?id=812911
Calculate vm uptime by subtracting process starttime from system uptime:
Almost equivalent to:
echo $(($(($(awk '{print $1}' /proc/uptime)*1e9 - $(($(cut -d -f22
/proc/$PID/stat)*1e7/(1.0 * 1e9)))
---
include/libvirt/libvirt-domain.h | 1 +
src/qemu/qemu_driver.c | 32 +---
src/remote/remote_protocol.x | 1 +
src/remote_protocol-structs | 1 +
tools/virsh-domain-monitor.c | 8
5 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index 7be4219..2df0241 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -275,6 +275,7 @@ struct _virDomainInfo {
unsigned long memory; /* the memory in KBytes used by the
domain */
unsigned short nrVirtCpu; /* the number of virtual CPUs for the
domain */
unsigned long long cpuTime; /* the CPU time used in nanoseconds */
+unsigned long long upTime; /* the total uptime in nanoseconds */
};
The public structs can't be changed once they are released. The next
best place will be the bulk stats API that is extensible.
/**
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f07e4fb..0b5098f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1309,11 +1309,12 @@ static char
*qemuConnectGetCapabilities(virConnectPtr conn) {
static int
qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long
*vm_rss,
- pid_t pid, int tid)
+ pid_t pid, int tid, unsigned long long *upTime)
{
char *proc;
FILE *pidinfo;
-unsigned long long usertime = 0, systime = 0;
+unsigned long long usertime = 0, systime = 0, starttime = 0;
+double _uptime;
long rss = 0;
int cpu = 0;
int ret;
@@ -1337,13 +1338,29 @@ qemuGetProcessInfo(unsigned long long *cpuTime,
int *lastCpu, long *vm_rss,
/* pid - stime */
%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu
%llu
/* cutime - endcode */
- %*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u
+ %*d %*d %*d %*d %*d %*d %llu %*u %ld %*u %*u %*u
/* startstack - processor */
%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d,
- usertime, systime, rss, cpu) != 4) {
+ usertime, systime, starttime, rss, cpu) != 5) {
VIR_WARN(cannot parse process status data);
}
+ret = virAsprintf(proc, /proc/uptime);
This copies a static string using virAsprintf?
+if (ret 0)
+return -1;
+
+pidinfo = fopen(proc, r);
And uses it in a place where you can use static strings? That doesn't
make sense.
+VIR_FREE(proc);
+
+if (!pidinfo || fscanf(pidinfo, %lf %*f, _uptime) != 1) {
+VIR_WARN(cannot parse machine uptime data);
+}
+
+if (upTime)
+*upTime = 1000ull * 1000ull * 1000ull * _uptime -
+(1000ull * 1000ull * 1000ull * starttime)
+/ (unsigned long long)sysconf(_SC_CLK_TCK);
This certainly does not calculate uptime of the guest, merely just the
time since the process started. This will not work at least in these
cases:
1) When the VM is migrated to a different host
2) When the VM is saved
3) The uptime will not be accurate when the guest was paused
4) If the guest OS reboots, it's uptime restarts
Is storing the timestamp at which the VM was started and then taking a
diff with the current timestamp right way to go?
How is uptime defined in case of (2) and (3) ?
At first, I'd not call it uptime at all. If you want to return the
uptime of the guest you need to use the guest agent connection as you
need to ask the actual guest for it's uptime. Guessing it is always
wrong.
For the calculation you are doing you should invent a different name as
that definitely is not uptime and will be confused with guest uptime.
Additionally I don't think that the stat would be useful.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH4/3] qemu: add cpu feature check for host-passthrough
Signed-off-by: Luyao Huang lhu...@redhat.com
---
I forgot this place.
src/qemu/qemu_process.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 79f763e..5f418a9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4070,11 +4070,6 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
bool ret = false;
size_t i;
-/* no features are passed to QEMU with -cpu host
- * so it makes no sense to verify them */
-if (def-cpu def-cpu-mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
-return true;
-
switch (arch) {
case VIR_ARCH_I686:
case VIR_ARCH_X86_64:
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: Extract internals of processBlockJobEvent into a helper
Later on I'll be adding a condition that will allow to synchronise a
SYNC block job abort. The approach will require this code to be called
from two different places so it has to be extracted into a helper.
---
src/qemu/qemu_driver.c | 200 +
1 file changed, 104 insertions(+), 96 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f1cbc46..257dea8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4453,116 +4453,101 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
static void
-processBlockJobEvent(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- char *diskAlias,
- int type,
- int status)
+qemuBlockJobEventProcess(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ int type,
+ int status)
{
virObjectEventPtr event = NULL;
virObjectEventPtr event2 = NULL;
const char *path;
-virDomainDiskDefPtr disk;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainDiskDefPtr persistDisk = NULL;
bool save = false;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-VIR_DEBUG(Domain is not running);
-goto endjob;
-}
-
-disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias);
-
-if (disk) {
-/* Have to generate two variants of the event for old vs. new
- * client callbacks */
-if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT
-disk-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
-type = disk-mirrorJob;
-path = virDomainDiskGetSource(disk);
-event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
-event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type,
- status);
-
-/* If we completed a block pull or commit, then update the XML
- * to match. */
-switch ((virConnectDomainEventBlockJobStatus) status) {
-case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
-if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) {
-if (vm-newDef) {
-int indx = virDomainDiskIndexByName(vm-newDef, disk-dst,
-false);
-virStorageSourcePtr copy = NULL;
-
-if (indx = 0) {
-persistDisk = vm-newDef-disks[indx];
-copy = virStorageSourceCopy(disk-mirror, false);
-if (virStorageSourceInitChainElement(copy,
- persistDisk-src,
- true) 0) {
-VIR_WARN(Unable to update persistent definition
- on vm %s after block job,
- vm-def-name);
-virStorageSourceFree(copy);
-copy = NULL;
-persistDisk = NULL;
-}
-}
-if (copy) {
-virStorageSourceFree(persistDisk-src);
-persistDisk-src = copy;
+/* Have to generate two variants of the event for old vs. new
+ * client callbacks */
+if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT
+disk-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+type = disk-mirrorJob;
+path = virDomainDiskGetSource(disk);
+event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
+event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type, status);
+
+/* If we completed a block pull or commit, then update the XML
+ * to match. */
+switch ((virConnectDomainEventBlockJobStatus) status) {
+case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
+if (disk-mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) {
+if (vm-newDef) {
+int indx = virDomainDiskIndexByName(vm-newDef, disk-dst,
false);
+virStorageSourcePtr copy = NULL;
+
+if (indx = 0) {
+persistDisk = vm-newDef-disks[indx];
+copy = virStorageSourceCopy(disk-mirror, false);
+if (virStorageSourceInitChainElement(copy,
+ persistDisk-src,
+ true) 0) {
+VIR_WARN(Unable to update persistent definition
+ on vm %s after block job,
+ vm-def-name);
+virStorageSourceFree(copy);
+
[libvirt] [PATCH 0/3] For 1.2.14: Fix regression in synchronous block job ABORT/PIVOT
When a block job is terminated via the synchronous API the backing chain would be updated in a separate thread and thus allowed applications to get outdated data. This broke live snapshot merge on oVirt. Since the commit breaking this (see patch 3/3) was not released yet I'm asking to merge this during the freeze. Peter Krempa (3): qemu: processBlockJob: Don't unlock @vm twice qemu: Extract internals of processBlockJobEvent into a helper qemu: blockjob: Synchronously update backing chain in XML on ABORT/PIVOT src/conf/domain_conf.c | 16 +++- src/conf/domain_conf.h | 6 ++ src/qemu/qemu_driver.c | 246 src/qemu/qemu_process.c | 38 +--- 4 files changed, 169 insertions(+), 137 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: processBlockJob: Don't unlock @vm twice
Commit 1a92c719 moved code to handle block job events to a different function that is executed in a separate thread. The caller of processBlockJob handles locking and unlocking of @vm, so the we should not do it in the function itself. --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f07e4fb..f1cbc46 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4574,7 +4574,6 @@ processBlockJobEvent(virQEMUDriverPtr driver, VIR_WARN(Unable to update persistent definition on vm %s after block job, vm-def-name); } -virObjectUnlock(vm); virObjectUnref(cfg); if (event) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: blockjob: Synchronously update backing chain in XML on ABORT/PIVOT
When the synchronous pivot option is selected, libvirt would not update
the backing chain until the job was exitted. Some applications then
received invalid data as their job serialized first.
This patch removes polling to wait for the ABORT/PIVOT job completion
and replaces it with a condition. If a synchronous operation is
requested the update of the XML is executed in the job of the caller of
the synchronous request. Otherwise the monitor event callback uses a
separate worker to update the backing chain with a new job.
This is a regression since 1a92c719101e5bfa6fe2b78006ad04c7f075ea28
When the ABORT job is finished synchronously you get the following call
stack:
#0 qemuBlockJobEventProcess
#1 qemuDomainBlockJobImpl
#2 qemuDomainBlockJobAbort
#3 virDomainBlockJobAbort
While previously or while using the _ASYNC flag you'd get:
#0 qemuBlockJobEventProcess
#1 processBlockJobEvent
#2 qemuProcessEventHandler
#3 virThreadPoolWorker
---
src/conf/domain_conf.c | 16 +++-
src/conf/domain_conf.h | 6 ++
src/qemu/qemu_driver.c | 45 +++--
src/qemu/qemu_process.c | 38 +-
4 files changed, 65 insertions(+), 40 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9324de0..cd6ee22 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1289,9 +1289,22 @@ virDomainDiskDefNew(void)
if (VIR_ALLOC(ret) 0)
return NULL;
+
if (VIR_ALLOC(ret-src) 0)
-VIR_FREE(ret);
+goto error;
+
+if (virCondInit(ret-blockJobSyncCond) 0) {
+virReportSystemError(errno, %s, _(Failed to initialize condition));
+goto error;
+}
+
return ret;
+
+ error:
+virStorageSourceFree(ret-src);
+VIR_FREE(ret);
+
+return NULL;
}
@@ -1310,6 +1323,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def-product);
VIR_FREE(def-domain_name);
virDomainDeviceInfoClear(def-info);
+virCondDestroy(def-blockJobSyncCond);
VIR_FREE(def);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 608f61f..84e880a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -685,6 +685,12 @@ struct _virDomainDiskDef {
int mirrorState; /* enum virDomainDiskMirrorState */
int mirrorJob; /* virDomainBlockJobType */
+/* for some synchronous block jobs, we need to notify the owner */
+virCond blockJobSyncCond;
+int blockJobType; /* type of the block job from the event */
+int blockJobStatus; /* status of the finished block job */
+bool blockJobSync; /* the block job needs synchronized termination */
+
struct {
unsigned int cylinders;
unsigned int heads;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 257dea8..b37995b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16276,6 +16276,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
goto endjob;
if (mode == BLOCK_JOB_ABORT) {
+if (async !(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
+/* prepare state for event delivery */
+disk-blockJobStatus = -1;
+disk-blockJobSync = true;
+}
+
if ((flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)
!(async disk-mirror)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -16389,37 +16395,24 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
status);
event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type,
status);
-} else if (!(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
+} else if (disk-blockJobSync) {
/* XXX If the event reports failure, we should reflect
* that back into the return status of this API call. */
-while (1) {
-/* Poll every 50ms */
-static struct timespec ts = {
-.tv_sec = 0,
-.tv_nsec = 50 * 1000 * 1000ull };
-virDomainBlockJobInfo dummy;
-
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJobInfo(priv-mon, device, dummy, NULL);
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-ret = -1;
-
-if (ret = 0)
-break;
-
-virObjectUnlock(vm);
-nanosleep(ts, NULL);
-
-virObjectLock(vm);
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain is not running));
-ret = -1;
-break;
+while (disk-blockJobStatus == -1 disk-blockJobSync) {
+if (virCondWait(disk-blockJobSyncCond, vm-parent.lock)
0) {
+
Re: [libvirt] [RFC] Add uptime to API returning Dom Info
On Mon, Mar 30, 2015 at 03:29:40 +0530, Nehal J Wani wrote:
This is an attempt to fix: https://bugzilla.redhat.com/show_bug.cgi?id=812911
Calculate vm uptime by subtracting process starttime from system uptime:
Almost equivalent to:
echo $(($(($(awk '{print $1}' /proc/uptime)*1e9 - $(($(cut -d -f22
/proc/$PID/stat)*1e7/(1.0 * 1e9)))
---
include/libvirt/libvirt-domain.h | 1 +
src/qemu/qemu_driver.c | 32 +---
src/remote/remote_protocol.x | 1 +
src/remote_protocol-structs | 1 +
tools/virsh-domain-monitor.c | 8
5 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index 7be4219..2df0241 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -275,6 +275,7 @@ struct _virDomainInfo {
unsigned long memory; /* the memory in KBytes used by the domain */
unsigned short nrVirtCpu; /* the number of virtual CPUs for the domain
*/
unsigned long long cpuTime; /* the CPU time used in nanoseconds */
+unsigned long long upTime; /* the total uptime in nanoseconds */
};
The public structs can't be changed once they are released. The next
best place will be the bulk stats API that is extensible.
/**
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f07e4fb..0b5098f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1309,11 +1309,12 @@ static char *qemuConnectGetCapabilities(virConnectPtr
conn) {
static int
qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
- pid_t pid, int tid)
+ pid_t pid, int tid, unsigned long long *upTime)
{
char *proc;
FILE *pidinfo;
-unsigned long long usertime = 0, systime = 0;
+unsigned long long usertime = 0, systime = 0, starttime = 0;
+double _uptime;
long rss = 0;
int cpu = 0;
int ret;
@@ -1337,13 +1338,29 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int
*lastCpu, long *vm_rss,
/* pid - stime */
%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu
%llu
/* cutime - endcode */
- %*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u
+ %*d %*d %*d %*d %*d %*d %llu %*u %ld %*u %*u %*u
/* startstack - processor */
%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d,
- usertime, systime, rss, cpu) != 4) {
+ usertime, systime, starttime, rss, cpu) != 5) {
VIR_WARN(cannot parse process status data);
}
+ret = virAsprintf(proc, /proc/uptime);
This copies a static string using virAsprintf?
+if (ret 0)
+return -1;
+
+pidinfo = fopen(proc, r);
And uses it in a place where you can use static strings? That doesn't
make sense.
+VIR_FREE(proc);
+
+if (!pidinfo || fscanf(pidinfo, %lf %*f, _uptime) != 1) {
+VIR_WARN(cannot parse machine uptime data);
+}
+
+if (upTime)
+*upTime = 1000ull * 1000ull * 1000ull * _uptime -
+(1000ull * 1000ull * 1000ull * starttime)
+/ (unsigned long long)sysconf(_SC_CLK_TCK);
This certainly does not calculate uptime of the guest, merely just the
time since the process started. This will not work at least in these
cases:
1) When the VM is migrated to a different host
2) When the VM is saved
3) The uptime will not be accurate when the guest was paused
...
+
/* We got jiffies
* We want nanoseconds
* _SC_CLK_TCK is jiffies per second
@@ -1404,7 +1421,8 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
(info[i].cpu),
NULL,
vm-pid,
- priv-vcpupids[i]) 0) {
+ priv-vcpupids[i],
+ NULL) 0) {
virReportSystemError(errno, %s,
_(cannot get vCPU placement pCPU
time));
return -1;
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Add uptime to API returning Dom Info
On Mon, Mar 30, 2015 at 1:16 PM, Peter Krempa pkre...@redhat.com wrote:
On Mon, Mar 30, 2015 at 03:29:40 +0530, Nehal J Wani wrote:
This is an attempt to fix: https://bugzilla.redhat.com/show_bug.cgi?id=812911
Calculate vm uptime by subtracting process starttime from system uptime:
Almost equivalent to:
echo $(($(($(awk '{print $1}' /proc/uptime)*1e9 - $(($(cut -d -f22
/proc/$PID/stat)*1e7/(1.0 * 1e9)))
---
include/libvirt/libvirt-domain.h | 1 +
src/qemu/qemu_driver.c | 32 +---
src/remote/remote_protocol.x | 1 +
src/remote_protocol-structs | 1 +
tools/virsh-domain-monitor.c | 8
5 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index 7be4219..2df0241 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -275,6 +275,7 @@ struct _virDomainInfo {
unsigned long memory; /* the memory in KBytes used by the domain
*/
unsigned short nrVirtCpu; /* the number of virtual CPUs for the
domain */
unsigned long long cpuTime; /* the CPU time used in nanoseconds */
+unsigned long long upTime; /* the total uptime in nanoseconds */
};
The public structs can't be changed once they are released. The next
best place will be the bulk stats API that is extensible.
/**
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f07e4fb..0b5098f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1309,11 +1309,12 @@ static char
*qemuConnectGetCapabilities(virConnectPtr conn) {
static int
qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
- pid_t pid, int tid)
+ pid_t pid, int tid, unsigned long long *upTime)
{
char *proc;
FILE *pidinfo;
-unsigned long long usertime = 0, systime = 0;
+unsigned long long usertime = 0, systime = 0, starttime = 0;
+double _uptime;
long rss = 0;
int cpu = 0;
int ret;
@@ -1337,13 +1338,29 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int
*lastCpu, long *vm_rss,
/* pid - stime */
%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu
%llu
/* cutime - endcode */
- %*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u
+ %*d %*d %*d %*d %*d %*d %llu %*u %ld %*u %*u %*u
/* startstack - processor */
%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d,
- usertime, systime, rss, cpu) != 4) {
+ usertime, systime, starttime, rss, cpu) != 5) {
VIR_WARN(cannot parse process status data);
}
+ret = virAsprintf(proc, /proc/uptime);
This copies a static string using virAsprintf?
+if (ret 0)
+return -1;
+
+pidinfo = fopen(proc, r);
And uses it in a place where you can use static strings? That doesn't
make sense.
+VIR_FREE(proc);
+
+if (!pidinfo || fscanf(pidinfo, %lf %*f, _uptime) != 1) {
+VIR_WARN(cannot parse machine uptime data);
+}
+
+if (upTime)
+*upTime = 1000ull * 1000ull * 1000ull * _uptime -
+(1000ull * 1000ull * 1000ull * starttime)
+/ (unsigned long long)sysconf(_SC_CLK_TCK);
This certainly does not calculate uptime of the guest, merely just the
time since the process started. This will not work at least in these
cases:
1) When the VM is migrated to a different host
2) When the VM is saved
3) The uptime will not be accurate when the guest was paused
Is storing the timestamp at which the VM was started and then taking a
diff with the current timestamp right way to go?
How is uptime defined in case of (2) and (3) ?
...
+
/* We got jiffies
* We want nanoseconds
* _SC_CLK_TCK is jiffies per second
@@ -1404,7 +1421,8 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
(info[i].cpu),
NULL,
vm-pid,
- priv-vcpupids[i]) 0) {
+ priv-vcpupids[i],
+ NULL) 0) {
virReportSystemError(errno, %s,
_(cannot get vCPU placement
pCPU time));
return -1;
Peter
--
Nehal J Wani
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: add a check for non-migratable cpu flags
If user set this feature in vm xml:
cpu mode='host-passthrough'
feature policy='require' name='invtsc'/
/cpu
we won't report error when do migrate.
So remove def-cpu-mode check, because we can set feature for
host-passthrough model now.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d34bb02..9c76e79 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2004,7 +2004,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver,
virDomainObjPtr vm,
return false;
}
-if (def-cpu def-cpu-mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
+if (def-cpu) {
for (i = 0; i def-cpu-nfeatures; i++) {
virCPUFeatureDefPtr feature = def-cpu-features[i];
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] docs: fix a small xml error in docs
Signed-off-by: Luyao Huang lhu...@redhat.com --- docs/formatdomain.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b496c3..577d647 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1019,7 +1019,7 @@ .../pre pre - lt;cpu mode='host-passthrough'/gt; + lt;cpu mode='host-passthrough'gt; lt;feature policy='disable' name='lahf_lm'/gt; .../pre -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] The risk of hanging when shutdown instance.
On 2015/3/28 18:06, Rui Chen wrote: Thank you for reply, Chris. 2015-03-27 23:15 GMT+08:00 Chris Friesen chris.frie...@windriver.com mailto:chris.frie...@windriver.com: On 03/26/2015 07:44 PM, Rui Chen wrote: Yes, you are right, but we found our instance hang at first dom.shutdown() call, if the dom.shutdown() don't return, there is no chance to execute dom.destroy(), right? Correct. The code is written assuming dom.shutdown() cannot block indefinitely. The libvirt docs at https://libvirt.org/html/__libvirt-libvirt-domain.html#__virDomainShutdown https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainShutdown say ...this command returns as soon as the shutdown request is issued rather than blocking until the guest is no longer running. If dom.shutdown() blocks indefinitely, then that's a libvirt bug. Chris The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] qemuProcessHook: Call virNuma*() iff needed
On Fri, Mar 27, 2015 at 03:26:53PM +0100, Michal Privoznik wrote:
Once upon a time, there was a little domain. And the domain was pinned
onto a NUMA node and hasn't fully allocated its memory:
memory unit='KiB'2355200/memory
currentMemory unit='KiB'1048576/currentMemory
numatune
memory mode='strict' nodeset='0'/
/numatune
Oh little me, said the domain, what will I do with so few memory.
s/few/little/
If I only had a few megabytes more. But the old admin noticed
whimpering, barely audible to untrained human ear. And good admin he
the whimpering
the good admin (?)
was, he gave the domain yet more memory. But the old NUMA topology
witch forbidden to allocate more memory on the node zero. So he
forbidden - forbade or forbid
decided to allocate it on a different node:
virsh # numatune little_domain --nodeset 0-1
virsh # setmem little_domain 2355200
The little domain was happy. For a while. Until bad, sharp teeth
a bad? the bad?
shaped creature came. Every process in the system was afraid of him.
The OOM Killer they called him. Oh no, he's after the little domain.
There's no escape.
Do you kids know why? Because when the little domain was born, her
father, Libvirt, called numa_set_membind(). So even if the admin
allowed her to allocate memory from other nodes in the cgroups, the
membind() forbid it.
So what's the lesson? Libvirt should rely on cgroups, whenever
possible and use numa_set_membind() as the last ditch effort.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_process.c | 32
1 file changed, 28 insertions(+), 4 deletions(-)
I don't have much experience proofreading children's books,
but the logic looks okay to me.
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 79f763e..cba042d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3154,6 +3154,7 @@ static int qemuProcessHook(void *data)
int fd;
virBitmapPtr nodeset = NULL;
virDomainNumatuneMemMode mode;
+bool doNuma = true;
/* This method cannot use any mutexes, which are not
* protected across fork()
@@ -3185,11 +3186,34 @@ static int qemuProcessHook(void *data)
goto cleanup;
mode = virDomainNumatuneGetMode(h-vm-def-numa, -1);
-nodeset = virDomainNumatuneGetNodeset(h-vm-def-numa,
- priv-autoNodeset, -1);
-if (virNumaSetupMemoryPolicy(mode, nodeset) 0)
-goto cleanup;
+if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+virCgroupPtr cgroup = NULL;
+
+/* Create dummy cgroup ... */
+if (virCgroupNewSelf(cgroup) 0)
+goto cleanup;
The domain's cgroup is accessible under priv-cgroup here, you can use
that one instead.
+
+/* ... just to detect if cpuset cgroup is present */
+if (virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
+/* Because if it's not, we must use virNuma* APIs to bind
+ * memory onto desired nodes. CGroup way is preferred, as
+ * it allows runtime tuning, while virNuma - well, once
+ * set and child (qemu) is exec()-ed, we can't do
+ * anything about the settings. virNuma* does not take
+ * any PID argument after all. */
Can this comment be shortened?
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] storage: Introduce storagePoolUpdateAllState function
On 03/25/2015 06:42 PM, John Ferlan wrote:
the 'checkPool' block has been moved to storagePoolUpdateAllState.
move the checkPool block into storageDriverInitialize. This ensures
activating pools earlier allowing domains using volumes from a pool to
be able to find the volume during qemuProcessReconnect ...
storagePoolUpdateAllState is called only from storageDriverInitialize...
moving checkPool to storageDriverInitialize would result in copying the
whole (65 lines) UpdateAllState body which I think is not necessary.
static void
+storagePoolUpdateAllState(void)
+{
+size_t i;
+bool active = false;
+
+for (i = 0; i driver-pools.count; i++) {
+virStoragePoolObjPtr pool = driver-pools.objs[i];
+virStorageBackendPtr backend;
+
+virStoragePoolObjLock(pool);
+if (!virStoragePoolObjIsActive(pool)) {
+virStoragePoolObjUnlock(pool);
+continue;
+}
+
+if ((backend = virStorageBackendForType(pool-def-type)) == NULL) {
+VIR_ERROR(_(Missing backend %d), pool-def-type);
+virStoragePoolObjUnlock(pool);
+continue;
+}
Redundant check with DriverAutostart and since that's the only place
that calls us I think dropping the VIR_ERROR is fine...
storageDriverInitialize is the only place that calls UpdateAllState.
Based on the fact that 'Initialize' is executed before 'Autostart' I
think it might be better to remove the error check from 'Autostart' instead.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] The risk of hanging when shutdown instance.
On 30.03.2015 11:28, zhang bo wrote: On 2015/3/28 18:06, Rui Chen wrote: snip/ The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) Yep, the documentation could be better in that respect. I've proposed a patch on the libvirt upstream list: https://www.redhat.com/archives/libvir-list/2015-March/msg01533.html Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On Mon, Mar 30, 2015 at 01:02:49PM +0200, Ján Tomko wrote: These cannot be represented in XML. We have been stripping them, but only if the string had characters that needed escaping: ' Extend the strcspn check to include control codes, and strip them even if we don't do any escaping. https://bugzilla.redhat.com/show_bug.cgi?id=1184131 https://bugzilla.redhat.com/show_bug.cgi?id=1066564 --- src/util/virbuffer.c | 14 +++--- tests/virbuftest.c | 49 + 2 files changed, 60 insertions(+), 3 deletions(-) ACK, according to XML documentation characters below 0x20 except 0x09, 0x0A and 0x0D are forbidden. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On 03/30/2015 05:02 AM, Ján Tomko wrote: These cannot be represented in XML. Yes they can, via entities. DV would know for sure, but I think that #x01; is the entity for the C byte '\1'. We have been stripping them, but only if the string had characters that needed escaping: ' Extend the strcspn check to include control codes, and strip them even if we don't do any escaping. NACK. Stripping control codes from a volume name represents the wrong name. We need to escape the problematic bytes, rather than strip them. https://bugzilla.redhat.com/show_bug.cgi?id=1184131 https://bugzilla.redhat.com/show_bug.cgi?id=1066564 --- src/util/virbuffer.c | 14 +++--- tests/virbuftest.c | 49 + 2 files changed, 60 insertions(+), 3 deletions(-) As there are real bugs that will be fixed once we use the correct entities, I'm looking forward to v2. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Few NUMA fixes
On Fri, Mar 27, 2015 at 03:26:48PM +0100, Michal Privoznik wrote: The heart of the patchset are the last two patches. Long story short, if a domain is configured to be pinned onto a set of NUMA nodes stricly, we use both CGroups and numa_set_membind(). While we can change the former later on a user's wish, we can't change the latter. Therefore, any subsequent memory enlargement (e.g. via virsh setmem $dom) will result in memory still being allocated from old NUMA nodes and OOM killer interference is likely to occur. Michal Privoznik (6): virCgroupNewPartition: Fix comment virCgroupNew: Enhance debug message virCgroupController: Check the enum fits into 'int' qemuDomainGetNumaParameters: Check for the correct CGroup controller qemuProcessHook: Call virNuma*() iff needed virLXCControllerSetupResourceLimits: Call virNuma*() iff needed ACK to patches 1-4, safe during the freeze. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] qemuProcessHook: Call virNuma*() iff needed
On 30.03.2015 15:15, Ján Tomko wrote:
On Fri, Mar 27, 2015 at 03:26:53PM +0100, Michal Privoznik wrote:
Once upon a time, there was a little domain. And the domain was pinned
onto a NUMA node and hasn't fully allocated its memory:
memory unit='KiB'2355200/memory
currentMemory unit='KiB'1048576/currentMemory
numatune
memory mode='strict' nodeset='0'/
/numatune
Oh little me, said the domain, what will I do with so few memory.
s/few/little/
If I only had a few megabytes more. But the old admin noticed
whimpering, barely audible to untrained human ear. And good admin he
the whimpering
the good admin (?)
was, he gave the domain yet more memory. But the old NUMA topology
witch forbidden to allocate more memory on the node zero. So he
forbidden - forbade or forbid
decided to allocate it on a different node:
virsh # numatune little_domain --nodeset 0-1
virsh # setmem little_domain 2355200
The little domain was happy. For a while. Until bad, sharp teeth
a bad? the bad?
shaped creature came. Every process in the system was afraid of him.
The OOM Killer they called him. Oh no, he's after the little domain.
There's no escape.
Do you kids know why? Because when the little domain was born, her
father, Libvirt, called numa_set_membind(). So even if the admin
allowed her to allocate memory from other nodes in the cgroups, the
membind() forbid it.
So what's the lesson? Libvirt should rely on cgroups, whenever
possible and use numa_set_membind() as the last ditch effort.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_process.c | 32
1 file changed, 28 insertions(+), 4 deletions(-)
I don't have much experience proofreading children's books,
but the logic looks okay to me.
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 79f763e..cba042d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3154,6 +3154,7 @@ static int qemuProcessHook(void *data)
int fd;
virBitmapPtr nodeset = NULL;
virDomainNumatuneMemMode mode;
+bool doNuma = true;
/* This method cannot use any mutexes, which are not
* protected across fork()
@@ -3185,11 +3186,34 @@ static int qemuProcessHook(void *data)
goto cleanup;
mode = virDomainNumatuneGetMode(h-vm-def-numa, -1);
-nodeset = virDomainNumatuneGetNodeset(h-vm-def-numa,
- priv-autoNodeset, -1);
-if (virNumaSetupMemoryPolicy(mode, nodeset) 0)
-goto cleanup;
+if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+virCgroupPtr cgroup = NULL;
+
+/* Create dummy cgroup ... */
+if (virCgroupNewSelf(cgroup) 0)
+goto cleanup;
The domain's cgroup is accessible under priv-cgroup here, you can use
that one instead.
Good point, I though that CGroups are created later in the process. But
due to handshaking with child, we are guaranteed that we can access
priv-cgroup. Will send v2 in a while.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH 0/3] libxl: domain destroy fixes
On Wed, Mar 25, 2015 at 02:08:33PM -0600, Jim Fehlig wrote: This small series of patches fixes some issues wrt domain destroy in the libxl driver. The primary motivation for this work is to prevent locking the virDomainObj during long running destroy operations on large memory domains. Patch 1 moves job acquisition from libxlDomainStart to it's callers so they have more control over when the job is acquired. Patch 2 fixes a few spots where we never acquired a job during domain destroy. Patch 3 contains the interesting change, where the virDomainObj is unlocked during the long-running destroy operation. This series wraps up my work to improve parallel OpenStack Tempest runs against the libxl driver. With libvirt.git master + this series + a patched libxl [1], I've successfully run a reproducer that was hitting the same issues encountered by Tempest. [1] libxl commits from xen.git: 93699882d, f1335f0d, 4783c99a, 1c91d6fba, and 188e9c54. I'll contact the stable branch maintainers and ask them to include these commits in the next Xen 4.4.x and 4.5.x releases. Hi, I gave a try to this series with OpenStack Tempest. And it is much better. Thanks! -- Anthony PERARD -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On Mon, Mar 30, 2015 at 07:06:45AM -0600, Eric Blake wrote: On 03/30/2015 05:02 AM, Ján Tomko wrote: These cannot be represented in XML. Yes they can, via entities. DV would know for sure, but I think that #x01; is the entity for the C byte '\1'. Only in XML 1.1. For XML 1.0: http://www.w3.org/TR/xml/#dt-charref Well-formedness constraint: Legal Character Characters referred to using character references MUST match the production for Char. Which is: http://www.w3.org/TR/xml/#NT-Char [2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x1-#x10]/* any Unicode character, excluding the surrogate blocks, FFFE, and . */ Both libvirt and virt-xml-validate choke on those entities error: (domain_definition):2: xmlParseCharRef: invalid xmlChar value 1 namef21#x01;/name DV was the one who wrote the code to skip over control characters in commit b36f453a581f27a4a43558978724a52df32045bb (v0.3.0~1) new function virBufferEscapeString() to format a string while escaping its content for XML, and apply it to a couple of obvious places, should fix bug #206653 It was the optimization in commit 0af02cb2e8d8192958735880e135ab69beb437c5 (v0.8.6~57) buf: Simplify virBufferEscapeString which broke this for strings that do have control codes, but not escapable characters. We have been stripping them, but only if the string had characters that needed escaping: ' Extend the strcspn check to include control codes, and strip them even if we don't do any escaping. NACK. Stripping control codes from a volume name represents the wrong name. We need to escape the problematic bytes, rather than strip them. https://bugzilla.redhat.com/show_bug.cgi?id=1184131 https://bugzilla.redhat.com/show_bug.cgi?id=1066564 --- src/util/virbuffer.c | 14 +++--- tests/virbuftest.c | 49 + 2 files changed, 60 insertions(+), 3 deletions(-) As there are real bugs that will be fixed once we use the correct entities, I'm looking forward to v2. This fixes the real bug of libvirt generating unparsable XML, which breaks creation of any VMs in virt-manager. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ceph rbd key
Is there a way that qemu takes the key of ceph rbd from a file rather than reading it from the file cinder.conf? Going from something like rbd_secret_uuid=UUID key to something like rbd_secret_uuid=/path/to/key (to restrict the access rights to the file and avoid the secret_uuid to be readable from anyone with ps kind of command) Regards, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] interface: allow multiple IPv4 addresses in interface XML
On Thu, Mar 26, 2015 at 08:18:07PM -0400, Laine Stump wrote: An upcoming netcf release will support multiple ipv4 addresses, so let's loosen up libvirt's interface.rng to allow it. --- docs/schemas/interface.rng | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) ACK after release, unless you have a reason for that, but I guess there is no need to push it now (even though there's nothing that could be broken). pgpcO4QUOx_HT.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virDomainShutdown*: Describe blocking behaviour
The description to both virDomainShutdown() and virDomainShutdownFlags() is quite misleading when it comes to blocking behaviour of these two APIs. Firstly, we support many shutdown methods, from signalizing an ACPI event, through sending a signal to guest agent assisted shutdown. Some of these methods make the API return immediately, while others block the API until domain is actually shut of. And since virDomainShutdown() is equivalent to calling virDomainShutdownFlags(0), it's up to each driver which methods to try. So the bare virDomainShutdown() may block or may return immediately at the same time. I know, it's confusing, but at least let users know. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/libvirt-domain.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index f1608dc..03b342f 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1474,9 +1474,12 @@ virDomainScreenshot(virDomainPtr domain, * 'on_poweroff' XML setting resulting in a domain that reboots instead of * shutting down. For guests that react to a shutdown request, the differences * from virDomainDestroy() are that the guests disk storage will be in a - * stable state rather than having the (virtual) power cord pulled, and - * this command returns as soon as the shutdown request is issued rather - * than blocking until the guest is no longer running. + * stable state rather than having the (virtual) power cord pulled. It's up to + * hypervisor's driver implementation what methods of + * virDomainShutdownFlagValues are tried and in which order. As described in + * virDomainShutdownFlags, this call may return immediately after the shutdown + * request is send, or it may block indefinitely long, until the domain is + * actually shut off. * * If the domain is transient and has any snapshot metadata (see * virDomainSnapshotNum()), then that metadata will automatically @@ -1540,7 +1543,9 @@ virDomainShutdown(virDomainPtr domain) * and a hypervisor is not required to support all methods. * * To use guest agent (VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) the domain XML - * must have channel configured. + * must have channel configured. Moreover, depending on underlying + * hypervisor used, passing this flag may block the API until the domain + * is shut off (which is not guaranteed to happen anyway). * * Returns 0 in case of success and -1 in case of failure. */ -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Strip control codes in virBufferEscapeString
These cannot be represented in XML.
We have been stripping them, but only if the string had
characters that needed escaping: '
Extend the strcspn check to include control codes, and strip
them even if we don't do any escaping.
https://bugzilla.redhat.com/show_bug.cgi?id=1184131
https://bugzilla.redhat.com/show_bug.cgi?id=1066564
---
src/util/virbuffer.c | 14 +++---
tests/virbuftest.c | 49 +
2 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 706dbfa..3d13c90 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -438,6 +438,13 @@ virBufferEscapeString(virBufferPtr buf, const char
*format, const char *str)
int len;
char *escaped, *out;
const char *cur;
+const char forbidden_characters[] = {
+0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+/*\t*/ /*\n*/ 0x0B, 0x0C, /*\r*/ 0x0E, 0x0F, 0x10,
+0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18,
+0x19, '','','\'', '','',
+'\0'
+};
if ((format == NULL) || (buf == NULL) || (str == NULL))
return;
@@ -446,7 +453,7 @@ virBufferEscapeString(virBufferPtr buf, const char *format,
const char *str)
return;
len = strlen(str);
-if (strcspn(str, '\) == len) {
+if (strcspn(str, forbidden_characters) == len) {
virBufferAsprintf(buf, format, str);
return;
}
@@ -490,8 +497,7 @@ virBufferEscapeString(virBufferPtr buf, const char *format,
const char *str)
*out++ = 'o';
*out++ = 's';
*out++ = ';';
-} else if (((unsigned char)*cur = 0x20) || (*cur == '\n') || (*cur ==
'\t') ||
- (*cur == '\r')) {
+} else if (!strchr(forbidden_characters, *cur)) {
/*
* default case, just copy !
* Note that character over 0x80 are likely to give problem
@@ -499,6 +505,8 @@ virBufferEscapeString(virBufferPtr buf, const char *format,
const char *str)
* it's hard to handle properly we have to assume it's UTF-8 too
*/
*out++ = *cur;
+} else {
+/* silently ignore control characters */
}
cur++;
}
diff --git a/tests/virbuftest.c b/tests/virbuftest.c
index 21cb18b..10398d5 100644
--- a/tests/virbuftest.c
+++ b/tests/virbuftest.c
@@ -349,6 +349,39 @@ testBufAddStr(const void *opaque ATTRIBUTE_UNUSED)
static int
+testBufEscapeStr(const void *opaque ATTRIBUTE_UNUSED)
+{
+const struct testBufAddStrData *data = opaque;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *actual;
+int ret = -1;
+
+virBufferAddLit(buf, c\n);
+virBufferAdjustIndent(buf, 2);
+virBufferEscapeString(buf, el%s/el\n, data-data);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /c);
+
+if (!(actual = virBufferContentAndReset(buf))) {
+TEST_ERROR(buf is empty);
+goto cleanup;
+}
+
+if (STRNEQ_NULLABLE(actual, data-expect)) {
+TEST_ERROR(testBufEscapeStr(): Strings don't match:\n);
+virtTestDifference(stderr, data-expect, actual);
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(actual);
+return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -379,6 +412,22 @@ mymain(void)
DO_TEST_ADD_STR(a/\n, c\n a/\n/c);
DO_TEST_ADD_STR(b\n a/\n/b\n, c\n b\na/\n
/b\n/c);
+#define DO_TEST_ESCAPE(data, expect) \
+do { \
+struct testBufAddStrData info = { data, expect }; \
+if (virtTestRun(Buf: EscapeStr, testBufEscapeStr, info) 0) \
+ret = -1; \
+} while (0)
+
+DO_TEST_ESCAPE(td/tdtd/td,
+ c\n
ellt;tdgt;lt;/tdgt;lt;tdgt;lt;/tdgt;/el\n/c);
+DO_TEST_ESCAPE(\007\\\x15,
+ c\n elquot;amp;amp;quot;/el\n/c);
+DO_TEST_ESCAPE(,,'..',,,
+ c\n el,,apos;..apos;,,/el\n/c);
+DO_TEST_ESCAPE(\x01\x01\x02\x03\x05\x08,
+ c\n el/el\n/c);
+
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.0.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix forget jump to clean up when set a big bandwidth
On Fri, Mar 27, 2015 at 17:56:29 +0800, Luyao Huang wrote: We already have a check for this, just add a jump to cleanup and change to use vshError instead of virReportError. Signed-off-by: Luyao Huang lhu...@redhat.com --- tools/virsh-domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) I've reworded the commit message and pushed the patch. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: blockCopy: Pass adjusted bandwidth when called via blockRebase
The block copy API takes the speed in bytes/s rather than MiB/s that was the prior approach in virDomainBlockRebase. We correctly converted the speed to bytes/s in the old API but we still called the common helper virDomainBlockCopyCommon with the unadjusted variable. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207122 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4d05221..6700fc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16811,7 +16811,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, flags = (VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT); ret = qemuDomainBlockCopyCommon(vm, dom-conn, path, dest, -bandwidth, 0, 0, flags, true); +speed, 0, 0, flags, true); dest = NULL; cleanup: -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On 03/30/2015 09:50 AM, Daniel Veillard wrote: NACK. Stripping control codes from a volume name represents the wrong name. We need to escape the problematic bytes, rather than strip them. you can't escape them with a CharRef for sure http://www.w3.org/TR/REC-xml/#wf-Legalchar Characters referred to using character references must match the production for Char. That time Ján is right :-) Ouch. Then how do we represent the name of a storage volume, when the file system allows arbitrary bytes including control characters, in the volume name, but where we are restricted to only using valid XML? Do we just silently ignore such files as impossible volumes that libvirt cannot manage? (I'd rather omit such a volume from the list in the pool, than silently munge its name into something incorrect) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockCopy: Pass adjusted bandwidth when called via blockRebase
On Mon, Mar 30, 2015 at 10:54:14 -0600, Eric Blake wrote: On 03/30/2015 09:41 AM, Peter Krempa wrote: The block copy API takes the speed in bytes/s rather than MiB/s that was the prior approach in virDomainBlockRebase. We correctly converted the speed to bytes/s in the old API but we still called the common helper virDomainBlockCopyCommon with the unadjusted variable. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207122 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) D'oh on my part. Probably snuck in while I was rebasing across several different attempts for the best normalization approach. It's a shame that the compiler/optimizer doesn't warn that the value is not used after the assignment. Pushed; Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On 03/30/2015 12:56 PM, Eric Blake wrote: On 03/30/2015 09:50 AM, Daniel Veillard wrote: NACK. Stripping control codes from a volume name represents the wrong name. We need to escape the problematic bytes, rather than strip them. you can't escape them with a CharRef for sure http://www.w3.org/TR/REC-xml/#wf-Legalchar Characters referred to using character references must match the production for Char. That time Ján is right :-) Ouch. Then how do we represent the name of a storage volume, when the file system allows arbitrary bytes including control characters, in the volume name, but where we are restricted to only using valid XML? Do we just silently ignore such files as impossible volumes that libvirt cannot manage? (I'd rather omit such a volume from the list in the pool, than silently munge its name into something incorrect) I'd say just make a follow up patch/bz to reject unrepresentable filenames before they hit the XML. It's a much less serious problem IMO - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockCopy: Pass adjusted bandwidth when called via blockRebase
On 03/30/2015 09:41 AM, Peter Krempa wrote: The block copy API takes the speed in bytes/s rather than MiB/s that was the prior approach in virDomainBlockRebase. We correctly converted the speed to bytes/s in the old API but we still called the common helper virDomainBlockCopyCommon with the unadjusted variable. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207122 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) D'oh on my part. Probably snuck in while I was rebasing across several different attempts for the best normalization approach. ACK. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4d05221..6700fc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16811,7 +16811,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, flags = (VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT); ret = qemuDomainBlockCopyCommon(vm, dom-conn, path, dest, -bandwidth, 0, 0, flags, true); +speed, 0, 0, flags, true); dest = NULL; cleanup: -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] conf: Rename virDomainHasDiskMirror and detect block jobs properly
On 03/30/2015 12:50 PM, Peter Krempa wrote: From: Shanzhi Yu s...@redhat.com virDomainHasDiskMirror() currently detects only jobs that add the mirror elements. Since some operations like migration are interlocked by existing block jobs on the given domain the check needs to be instrumented to check regular jobs too. This patch renames virDomainHasDiskMirror to virDomainHasDiskBlockjob and adds an argument that allows to select that it returns true only for block copy jobs as those interlock making the domain persistent. Other two uses trigger on any block job type. Signed-off-by: Shanzhi Yu s...@redhat.com Signed-off-by: Peter Krempa pkre...@redhat.com --- src/conf/domain_conf.c| 25 - src/conf/domain_conf.h| 3 ++- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c| 6 +++--- src/qemu/qemu_migration.c | 2 +- 5 files changed, 27 insertions(+), 11 deletions(-) ACK; series should be safe for freeze as a bug fix. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.2.14
On Fri, Mar 27, 2015 at 12:12:42 +0800, Daniel Veillard wrote: I have tagged a release candidate 1 in git and generated signed tarballs and rpms at the usual place: ftp://libvirt.org/libvirt/ This seems to work normally in my limited testing but others need to give it a serious try ! I think I will do an RC2 on Monday, and if everything look file the final release will be on April 1st. Please note that the series https://www.redhat.com/archives/libvir-list/2015-March/msg01524.html should go in before the release since we introduced a regression for oVirt where the backing chain is not updated timely and oVirts live snapshot merge code fails due to unfortunate serialization of threads. Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] qemu: Fix issues with maxMemory in qemuDomainSetMemoryFlags()
From: Luyao Huang lhu...@redhat.com
qemuDomainSetMemoryFlags() would allow to set the initial memory greater
than the maxMemory field. While the configuration would not work as
memory hotplug requires NUMA to be enabled and the
qemuDomainSetMemoryFlags() API does not work on NUMA guests this just
fixes a corner case.
The fix is still worth though as it allows to induce an invalid
configuration and make the VM vanish on libvirt restart.
Additionally this tweaks error message to be more accurate.
Signed-off-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Peter Krempa pkre...@redhat.com
---
Version 2 tweaks the error messages to be (possibly) more descriptive.
src/qemu/qemu_driver.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6700fc9..d15931c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2319,11 +2319,19 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
* is no way to change the individual node sizes with this API */
if (virDomainNumaGetNodeCount(persistentDef-numa) 0) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(maximum memory size of a domain with NUMA
+ _(initial memory size of a domain with NUMA
nodes cannot be modified with this API));
goto endjob;
}
+if (persistentDef-mem.max_memory
+persistentDef-mem.max_memory newmem) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(cannot set initial memory size biger than
+ the maximum memory size));
+goto endjob;
+}
+
virDomainDefSetMemoryInitial(persistentDef, newmem);
if (persistentDef-mem.cur_balloon newmem)
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH 0/3] libxl: domain destroy fixes
Jim Fehlig wrote: Konrad Rzeszutek Wilk wrote: On Wed, Mar 25, 2015 at 02:08:33PM -0600, Jim Fehlig wrote: This small series of patches fixes some issues wrt domain destroy in the libxl driver. The primary motivation for this work is to prevent locking the virDomainObj during long running destroy operations on large memory domains. Patch 1 moves job acquisition from libxlDomainStart to it's callers so they have more control over when the job is acquired. Patch 2 fixes a few spots where we never acquired a job during domain destroy. Patch 3 contains the interesting change, where the virDomainObj is unlocked during the long-running destroy operation. This series wraps up my work to improve parallel OpenStack Tempest runs against the libxl driver. With libvirt.git master + this series + a patched libxl [1], I've successfully run a reproducer that was hitting the same issues encountered by Tempest. [1] libxl commits from xen.git: 93699882d, f1335f0d, 4783c99a, 1c91d6fba, and 188e9c54. I'll contact the stable branch maintainers and ask them to include these commits in the next Xen 4.4.x and 4.5.x releases. Jim Fehlig (3): libxl: Move job acquisition in libxlDomainStart to callers libxl: acquire a job when destroying a domain libxl: drop virDomainObj lock when destroying a domain I am no expert at this- but I dug through the code to understand how the job and locking is done and now I am more comfortable with it. Since I am new to this I went through all of the the callsites (which used the job now) from the driver to make sure that there are no chained calls (one function calling another which also uses a mutex or job locking). I only found one culprit (libxlDomainAutoCoreDump being called from libxlDomainShutdownThread). Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Thanks for taking time to review the series and familiarize yourself with the libvirt libxl code! As mentioned, I squashed in your libxlDomainAutoCoreDump fix in 2/3. Do any other libvirt devs have time for a quick review? I'd like to push this series, and the dom0 ballooning fix, for 1.2.14 if there are no objections. The latter was reviewed by Stefano Stabellini before the freeze https://www.redhat.com/archives/libvir-list/2015-March/msg01248.html Ping! I'd like to include these fixes for 1.2.14. The patches have been Reviewed-by Konrad and Stefano. Anthony also responded today that his OpenStack Tempest runs are much happier https://www.redhat.com/archives/libvir-list/2015-March/msg01540.html Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: snapshot: Check for block jobs individually
On 03/30/2015 12:50 PM, Peter Krempa wrote: If any disk of a VM was involved in a (copy) block job we refused to do a snapshot. As not only copy jobs interlock snapshots and the interlocking is applicable to individual disks only we can make the check in a more individual fashion and interlock all block job types supported by libvirt. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1203628 --- src/qemu/qemu_driver.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] qemu: Finish fixing of interlocking domain ops with blockjobs
Peter Krempa (1): qemu: snapshot: Check for block jobs individually Shanzhi Yu (1): conf: Rename virDomainHasDiskMirror and detect block jobs properly src/conf/domain_conf.c| 25 - src/conf/domain_conf.h| 3 ++- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c| 19 +++ src/qemu/qemu_migration.c | 2 +- 5 files changed, 35 insertions(+), 16 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] conf: Rename virDomainHasDiskMirror and detect block jobs properly
From: Shanzhi Yu s...@redhat.com
virDomainHasDiskMirror() currently detects only jobs that add the mirror
elements. Since some operations like migration are interlocked by
existing block jobs on the given domain the check needs to be
instrumented to check regular jobs too.
This patch renames virDomainHasDiskMirror to virDomainHasDiskBlockjob
and adds an argument that allows to select that it returns true only for
block copy jobs as those interlock making the domain persistent.
Other two uses trigger on any block job type.
Signed-off-by: Shanzhi Yu s...@redhat.com
Signed-off-by: Peter Krempa pkre...@redhat.com
---
src/conf/domain_conf.c| 25 -
src/conf/domain_conf.h| 3 ++-
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c| 6 +++---
src/qemu/qemu_migration.c | 2 +-
5 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9324de0..16cdfa2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12263,15 +12263,30 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const
char *name)
return virDomainDiskRemove(def, idx);
}
-/* Return true if VM has at least one disk involved in a current block
- * copy/commit job (that is, with a mirror element in the disk xml). */
+/**
+ * virDomainHasBlockjob:
+ * @vm: domain object
+ * @copy_only: Reject only block copy job
+ *
+ * Return true if @vm has at least one disk involved in a current block
+ * copy/commit/pull job. If @copy_only is true this returns true only if the
+ * disk is involved in a block copy.
+ * */
bool
-virDomainHasDiskMirror(virDomainObjPtr vm)
+virDomainHasBlockjob(virDomainObjPtr vm,
+ bool copy_only)
{
size_t i;
-for (i = 0; i vm-def-ndisks; i++)
-if (vm-def-disks[i]-mirror)
+for (i = 0; i vm-def-ndisks; i++) {
+if (!copy_only
+vm-def-disks[i]-blockjob)
+return true;
+
+if (vm-def-disks[i]-mirror
+vm-def-disks[i]-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
return true;
+}
+
return false;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 608f61f..493fbba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2654,7 +2654,8 @@ int virDomainDiskSourceParse(xmlNodePtr node,
xmlXPathContextPtr ctxt,
virStorageSourcePtr src);
-bool virDomainHasDiskMirror(virDomainObjPtr vm);
+bool virDomainHasBlockjob(virDomainObjPtr vm,
+ bool copy_only);
int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5716ece..9e71b1a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -297,7 +297,7 @@ virDomainGraphicsTypeFromString;
virDomainGraphicsTypeToString;
virDomainGraphicsVNCSharePolicyTypeFromString;
virDomainGraphicsVNCSharePolicyTypeToString;
-virDomainHasDiskMirror;
+virDomainHasBlockjob;
virDomainHasNet;
virDomainHostdevCapsTypeToString;
virDomainHostdevDefAlloc;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6bc305e..eac04cf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7389,7 +7389,7 @@ static virDomainPtr
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
virObjectRef(vm);
def = NULL;
-if (virDomainHasDiskMirror(vm)) {
+if (virDomainHasBlockjob(vm, true)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
_(domain has active block job));
virDomainObjAssignDef(vm, NULL, false, NULL);
@@ -15239,8 +15239,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
-if (virDomainHasDiskMirror(vm)) {
-virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
+if (virDomainHasBlockjob(vm, false)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(domain has active block job));
goto cleanup;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d34bb02..8c45415 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1977,7 +1977,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver,
virDomainObjPtr vm,
}
-if (virDomainHasDiskMirror(vm)) {
+if (virDomainHasBlockjob(vm, false)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(domain has an active block job));
return false;
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: snapshot: Check for block jobs individually
If any disk of a VM was involved in a (copy) block job we refused to do
a snapshot. As not only copy jobs interlock snapshots and the
interlocking is applicable to individual disks only we can make the
check in a more individual fashion and interlock all block job types
supported by libvirt.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1203628
---
src/qemu/qemu_driver.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6700fc9..6bc305e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13928,6 +13928,14 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
virDomainSnapshotDiskDefPtr disk = def-disks[i];
virDomainDiskDefPtr dom_disk = vm-def-disks[i];
+if (disk-snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE
+dom_disk-blockjob) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(disk '%s' has an active block job),
+ disk-name);
+goto cleanup;
+}
+
switch ((virDomainSnapshotLocation) disk-snapshot) {
case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
found_internal = true;
@@ -14574,11 +14582,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
%s, _(domain is marked for auto destroy));
goto cleanup;
}
-if (virDomainHasDiskMirror(vm)) {
-virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
- _(domain has active block job));
-goto cleanup;
-}
if (!vm-persistent (flags VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] The risk of hanging when shutdown instance.
On 03/30/2015 06:08 AM, Michal Privoznik wrote: On 30.03.2015 11:28, zhang bo wrote: On 2015/3/28 18:06, Rui Chen wrote: snip/ The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) Yep, the documentation could be better in that respect. I've proposed a patch on the libvirt upstream list: https://www.redhat.com/archives/libvir-list/2015-March/msg01533.html I don't think a doc patch is right. If you don't pass any flags, then it is up to the hypervisor which method it will attempt (agent or ACPI). Yes, explicitly requesting an agent as the only method to attempt might be justifiable as a reason to block, but the overall API contract is to NOT block indefinitely. I think that rather than a doc patch, we need to fix the underlying bug, and guarantee that we return after a finite time even when the agent is involved. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Extract internals of processBlockJobEvent into a helper
On 03/30/2015 03:26 AM, Peter Krempa wrote: Later on I'll be adding a condition that will allow to synchronise a SYNC block job abort. The approach will require this code to be called from two different places so it has to be extracted into a helper. --- src/qemu/qemu_driver.c | 200 + 1 file changed, 104 insertions(+), 96 deletions(-) ACK; looks to be code motion with no immediate impact, so safe for freeze if 3/3 is approved. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: blockjob: Synchronously update backing chain in XML on ABORT/PIVOT
On 03/30/2015 03:26 AM, Peter Krempa wrote:
When the synchronous pivot option is selected, libvirt would not update
the backing chain until the job was exitted. Some applications then
s/exitted/exited/
received invalid data as their job serialized first.
This patch removes polling to wait for the ABORT/PIVOT job completion
and replaces it with a condition. If a synchronous operation is
requested the update of the XML is executed in the job of the caller of
the synchronous request. Otherwise the monitor event callback uses a
separate worker to update the backing chain with a new job.
This is a regression since 1a92c719101e5bfa6fe2b78006ad04c7f075ea28
unreleased, and therefore worth fixing during freeze.
When the ABORT job is finished synchronously you get the following call
stack:
#0 qemuBlockJobEventProcess
#1 qemuDomainBlockJobImpl
#2 qemuDomainBlockJobAbort
#3 virDomainBlockJobAbort
While previously or while using the _ASYNC flag you'd get:
#0 qemuBlockJobEventProcess
#1 processBlockJobEvent
#2 qemuProcessEventHandler
#3 virThreadPoolWorker
---
src/conf/domain_conf.c | 16 +++-
src/conf/domain_conf.h | 6 ++
src/qemu/qemu_driver.c | 45 +++--
src/qemu/qemu_process.c | 38 +-
4 files changed, 65 insertions(+), 40 deletions(-)
@@ -16389,37 +16395,24 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
status);
event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type,
status);
-} else if (!(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
+} else if (disk-blockJobSync) {
/* XXX If the event reports failure, we should reflect
* that back into the return status of this API call. */
Isn't this comment stale now? That is, since you are waiting for the
event to free the condition, and we are listening for both success and
failure events, we can now tell if the event reports failure.
Otherwise, looks correct to me. I hate that it is so close to the
release deadline, but I hate regressions more (which is why we have
freezes). I'd feel better if you got a second review, if you can
wrangle one up in time, but here's my:
ACK.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] storage: handle scsi/iscsi error paths better
Adjust error path logic for processing LU's in order to better ascertain the correct cause for failure. The first two patches are merely setting for the third one which determines that the pool definition used a non existent path in the configuration thus no 'real' targets were able to be created. John Ferlan (3): iscsi: Check for presence of error before creating new one scsi: Adjust return values from processLU scsi: Check for invalid target.path after processLU failure src/storage/storage_backend_iscsi.c | 5 +++-- src/storage/storage_backend_scsi.c | 13 +++-- 2 files changed, 14 insertions(+), 4 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] scsi: Check for invalid target.path after processLU failure
https://bugzilla.redhat.com/show_bug.cgi?id=1171933
After processing all the LU's and find no real LU's - check if perhaps
the cause of that was a poorly formed 'target.path'. The expection is
generally that the path is /dev/disk/by-path or at least something in /dev.
Although it's not impossible for the user to provide something. If they
do provide something it should be valid or we should just cause a failure
to start the pool with an appropriate message.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_scsi.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 2f1f5ed..c3a1892 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -467,6 +467,15 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr
pool,
if (!*found)
VIR_DEBUG(No LU found for pool %s, pool-def-name);
+if (!*found !virFileExists(pool-def-target.path)
+!STRPREFIX(pool-def-target.path, /dev)) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(No LUs found for pool '%s' target
+ path '%s' not found),
+ pool-def-name, pool-def-target.path);
+retval = -1;
+}
+
closedir(devicedir);
return retval;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] scsi: Adjust return values from processLU
Currently processLU returns a 0 when either a 'non disk/lun' volume or a processed and found disk/lun value. On return we set *found = true in either case. If we don't find any real LU's that could be indicative of some other problem that we may need to message. Therefore, only set the *found when we've successfully processed a LU. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 58e7e6d..2f1f5ed 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -412,7 +412,7 @@ processLU(virStoragePoolObjPtr pool, host, bus, target, lun); goto out; } -retval = 0; +retval = 1; VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully, host, bus, target, lun); @@ -460,7 +460,7 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr pool, VIR_DEBUG(Found possible LU '%s', lun_dirent-d_name); -if (processLU(pool, scanhost, bus, target, lun) == 0) +if (processLU(pool, scanhost, bus, target, lun) == 1) *found = true; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virDomainShutdown*: Describe blocking behaviour
On 03/30/2015 06:07 AM, Michal Privoznik wrote: The description to both virDomainShutdown() and virDomainShutdownFlags() is quite misleading when it comes to blocking behaviour of these two APIs. Firstly, we support many shutdown methods, from signalizing an ACPI event, through sending a signal to guest agent assisted shutdown. Some of these methods make the API return immediately, while others block the API until domain is actually shut of. And since virDomainShutdown() is s/of/off/ equivalent to calling virDomainShutdownFlags(0), it's up to each driver which methods to try. So the bare virDomainShutdown() may block or may return immediately at the same time. I know, it's confusing, but at least let users know. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/libvirt-domain.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) I don't think this is the right approach. It is okay to block for a small finite amount of time, but if the guest agent really is something that can hang, we should fix that code to return without waiting for the agent response. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] iscsi: Check for presence of error before creating new one
If virStorageBackendSCSIFindLUs fails, but the failure has an error
message - the iscsi code didn't honor that creating it's own wonderful
message such as error: Failed to find LUs on host 60: ... - not overly
helpful. Since a few of the called paths generate a message, check for
that before using that generic one.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_iscsi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 079c767..1dac238 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -147,8 +147,9 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
}
if (virStorageBackendSCSIFindLUs(pool, host) 0) {
-virReportSystemError(errno,
- _(Failed to find LUs on host %u), host);
+if (virGetLastError() == NULL)
+virReportSystemError(errno,
+ _(Failed to find LUs on host %u), host);
retval = -1;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: processBlockJob: Don't unlock @vm twice
On 03/30/2015 03:26 AM, Peter Krempa wrote: Commit 1a92c719 moved code to handle block job events to a different function that is executed in a separate thread. The caller of processBlockJob handles locking and unlocking of @vm, so the we should not do it in the function itself. --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) ACK; bug fix so safe for freeze. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f07e4fb..f1cbc46 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4574,7 +4574,6 @@ processBlockJobEvent(virQEMUDriverPtr driver, VIR_WARN(Unable to update persistent definition on vm %s after block job, vm-def-name); } -virObjectUnlock(vm); virObjectUnref(cfg); if (event) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] connect: ssh: Shall we remove the dependency of netcat ?
On 2015/3/28 0:25, Peter Krempa wrote: On Fri, Mar 27, 2015 at 10:54:26 +0800, zhang bo wrote: Too powerful? That is a ridiculous statement that probably originates from some kind of misunderstanding when creating a security policy or stuff like that. If a policy bans nc as powerful then it's missing on a lot of other options how to create listening or outgoing connections on arbitrary sockets. The only insecure part is that it does not use encryption, but that's a widely known fact about nc. Sorry for replying so late:) I tried to confirm the security fact the other days, unfortunately no clear answer gotten. What I meant was that the *network monitoring tools*, such as 'nc' and 'tcpdump', they are considered as dangerous for network security. They are prohibited in our company and some other organizations. I'm not quite sure whether the result that they're prohibited are directly related to their capability of monitoring network. But they actually got prohibited anyway. 3 So, is there any good substitution for netcat to realize qemu+ssh? Currently libvirt doesn't allow this, but I'm planning for a long time to introduce a standalone libvirt client binary (or perhaps add this as a mode to virsh) to replace the use of NC but that's due to other reasons: 1) nc doesn't know where session mode sockets are placed This is due to the fact that it depends on how libvirt is compiled. Currently the client side has to provide the path that is used at the remote side and those may not correspond. 2) errors reported when using the ssh connection transport are not helpful: NC is inherently bad at reporting what happened with the unix socket on the remote side. 3) getting rid of nc as a dependency This won't happen though ... old libvirt clients would not be able to connect to newer servers. In other words. I don't think libvirt will ever get rid of using nc but we can make stuff better by adding the internal remote client. Peter Thank you for the detailed reply, glad to hear that it's on schedule to be replaced someday. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Release of libvirt-1.2.14 candidate release 2
So I have just tagged in git and pushed the 1.2.14 candidate release 2, it is available as usual as signed tarballs and rpms from: ftp://libvirt.org/libvirt/ I did push the 3 patches from Peter because what is the point of a candidate release if it doesn't include the potentially controversial bits that we intend for the final release ? That doesn't prevent someone else than Eric to give the 2nd ACK for patch 3 of the series :-) So please git it a try, if needed we will just do an rc3 ! BTW is there regression testing done from oVirt using libvirt upstream on an automated basis ? If not that's something I could help pushing for assuming we have public reg tests for oVirt. At least it seems to work in my minimal own tests, but others should check, thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On Mon, Mar 30, 2015 at 10:56:11AM -0600, Eric Blake wrote: On 03/30/2015 09:50 AM, Daniel Veillard wrote: NACK. Stripping control codes from a volume name represents the wrong name. We need to escape the problematic bytes, rather than strip them. you can't escape them with a CharRef for sure http://www.w3.org/TR/REC-xml/#wf-Legalchar Characters referred to using character references must match the production for Char. That time Ján is right :-) Ouch. Then how do we represent the name of a storage volume, when the file system allows arbitrary bytes including control characters, in the volume name, but where we are restricted to only using valid XML? Do we just silently ignore such files as impossible volumes that libvirt cannot manage? (I'd rather omit such a volume from the list in the pool, than silently munge its name into something incorrect) Since if such an invalid CharRef were to hit libxml2 you would get a parser error and no result. So you can safely assume nobody ever has experienced those. Then you can try to push an additional patch doing a libvirt escaping but of only those problematic characters prior to the encoding in the XML. Then escape them back when reading from the XML to libvirt internals. This should not affect any deployed instance since they would be unparseable if that was the case. I would suggest using the same charref escaping but before passing to XML, e.g. real path: /foo\3bar libvirt encoded: /foo#3;bar XML encoded: /fooamp;#3;bar you also need to catch and give him special status real path: /foobar libvirt encoded: /foo#38;bar XML encoded: /fooamp;#38;bar after libvirt parsing you end up with /foo#3;bar and each time you see #numericsequence; you translate that to the equivalent UTF-8 character. Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x1-#x10] As a first approach, I would suggest just detecting bytes 1-8 0xB-0x1F and giving them the treatment, the probability of hitting surrogates in UTF-8 filesnames seems low enough that the patch should work in general. Whether using /foo#3;bar vs. /foo#0x3;bar is a matter of taste you only need to handle one IMHO. Add a little regression tests with all the lower caracter and use in the path and I think you're covered. Sounds too late for 1.2.14 though, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] The risk of hanging when shutdown instance.
On 2015/3/31 4:36, Eric Blake wrote: On 03/30/2015 06:08 AM, Michal Privoznik wrote: On 30.03.2015 11:28, zhang bo wrote: On 2015/3/28 18:06, Rui Chen wrote: snip/ The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) Yep, the documentation could be better in that respect. I've proposed a patch on the libvirt upstream list: https://www.redhat.com/archives/libvir-list/2015-March/msg01533.html I don't think a doc patch is right. If you don't pass any flags, then it is up to the hypervisor which method it will attempt (agent or ACPI). Yes, explicitly requesting an agent as the only method to attempt might be justifiable as a reason to block, but the overall API contract is to NOT block indefinitely. I think that rather than a doc patch, we need to fix the underlying bug, and guarantee that we return after a finite time even when the agent is involved. So, may we get to a final decision? :) Shall we timeout in virDomainShutdown() or leave it to openstack? The 2 solutions I can see are: 1) timeout in virDomainShutdown() and virDomainReboot(). in libvirt. 2) spawn a new thread to monitor the guest's status, if it's not shutoff after dom.shutdown() for a while, call dom.destroy() to force shut it down. in openstack. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Strip control codes in virBufferEscapeString
On Mon, Mar 30, 2015 at 07:06:45AM -0600, Eric Blake wrote: On 03/30/2015 05:02 AM, Ján Tomko wrote: These cannot be represented in XML. Yes they can, via entities. DV would know for sure, but I think that #x01; is the entity for the C byte '\1'. no they can't :-) A character must match prod Char, even when using a CharRef http://www.w3.org/TR/REC-xml/#NT-Char [2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x1-#x10]/* any Unicode character, excluding the surrogate blocks, FFFE, and . */ We have been stripping them, but only if the string had characters that needed escaping: ' Extend the strcspn check to include control codes, and strip them even if we don't do any escaping. NACK. Stripping control codes from a volume name represents the wrong name. We need to escape the problematic bytes, rather than strip them. you can't escape them with a CharRef for sure http://www.w3.org/TR/REC-xml/#wf-Legalchar Characters referred to using character references must match the production for Char. That time Ján is right :-) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
