Re: [libvirt] [PATCH] qemu: add a value check for granularity
On 27.03.2015 10:56, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1206479
From our manual of virsh and qemu side code, we know this
value must be power of 2, so instead of let qemu output error,
we can add a check when we file this value in qemuDomainBlockCopy.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2c55fb0..6d63317 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16871,6 +16871,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char
*disk, const char *destxml,
}
bandwidth = param-value.ul;
} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_COPY_GRANULARITY)) {
+if (param-value.ui !=
VIR_ROUND_UP_POWER_OF_TWO(param-value.ui)) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(granularity must be power of 2));
+goto cleanup;
+}
granularity = param-value.ui;
} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_COPY_BUF_SIZE)) {
buf_size = param-value.ul;
In fact, the virsh man page is not as precise as it could be either:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 63325ff..5d52761 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1039,10 +1039,10 @@ unlimited, but more likely would overflow; it is safer
to use 0 for that
purpose. Specifying Igranularity allows fine-tuning of the granularity that
will be copied when a dirty region is detected; larger values trigger less
I/O overhead but may end up copying more data overall (the default value is
usually correct); {+hypervisors may restrict+} this [-value must-]{+to+} be a
power of [-two.-]{+two or fall+}
{+within a certain range.+} Specifying Ibuf-size will control how much data
can
be simultaneously in-flight during the copy; larger values use more memory but
may allow faster completion (the default value is usually correct).
=item Bblockpull Idomain Ipath [Ibandwidth] [Ibase]
[I--wait [I--verbose] [I--timeout Bseconds] [I--async]]
I'm fixing the man page too, rewording the commit message a bit and ACKing.
Will push after the release.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-perl PATCH] Add VIR_FROM_THREAD constant
On 28.03.2015 12:49, John Ferlan wrote: Signed-off-by: John Ferlan jfer...@redhat.com --- Changes | 1 + Virt.xs | 1 + lib/Sys/Virt/Error.pm | 4 3 files changed, 6 insertions(+) diff --git a/Changes b/Changes index 7a2bc51..1849668 100644 --- a/Changes +++ b/Changes @@ -11,6 +11,7 @@ Revision history for perl module Sys::Virt - Add VIR_DOMAIN_PAUSED_STARTING_UP constant - Adapt to rename of virDomainIOThreadsInfoFree to virDomainIOThreadInfoFree - Adapt to rename of virDomainGetIOThreadsInfo to virDomainGetIOThreadInfo + - Add VIR_FROM_THREAD constant 1.2.13 2015-03-05 diff --git a/Virt.xs b/Virt.xs index 2138530..d01cf05 100644 --- a/Virt.xs +++ b/Virt.xs @@ -8104,6 +8104,7 @@ BOOT: REGISTER_CONSTANT(VIR_FROM_CRYPTO, FROM_CRYPTO); REGISTER_CONSTANT(VIR_FROM_FIREWALL, FROM_FIREWALL); REGISTER_CONSTANT(VIR_FROM_POLKIT, FROM_POLKIT); + REGISTER_CONSTANT(VIR_FROM_THREAD, FROM_THREAD); REGISTER_CONSTANT(VIR_ERR_OK, ERR_OK); diff --git a/lib/Sys/Virt/Error.pm b/lib/Sys/Virt/Error.pm index 2171bf2..e2fdbe1 100644 --- a/lib/Sys/Virt/Error.pm +++ b/lib/Sys/Virt/Error.pm @@ -378,6 +378,10 @@ The firewall helper APIs. The polkit authentication / authorization APIs +=item Sys::Virt::Error::FROM_THREAD + +The thread helper utils + =back =head2 ERROR CODE CONSTANTS ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: nodeinfo: Test F21 aarch64 on APM mustang
On 28.03.2015 19:31, Cole Robinson wrote: --- diff --git a/tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem b/tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem new file mode 12 index 000..758c291 --- /dev/null +++ b/tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem @@ -0,0 +1 @@ +../../../../bus/cpu \ No newline at end of file so this adds a symlink to ../../../../bus/cpu. However, the pointee does not exist: libvirt.git $ file tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem: broken symbolic link to ../../../../bus/cpu But, the file seems to be not needed anyway: libvirt.git/tests $ ../run strace nodeinfotest 21 | grep subsystem | wc -l 0 I think we should remove it. Oh, and there are some more files like this. Patch on its way. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add a value check for granularity
On 04/01/2015 04:12 PM, Michal Privoznik wrote:
On 27.03.2015 10:56, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1206479
From our manual of virsh and qemu side code, we know this
value must be power of 2, so instead of let qemu output error,
we can add a check when we file this value in qemuDomainBlockCopy.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2c55fb0..6d63317 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16871,6 +16871,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char
*disk, const char *destxml,
}
bandwidth = param-value.ul;
} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_COPY_GRANULARITY)) {
+if (param-value.ui != VIR_ROUND_UP_POWER_OF_TWO(param-value.ui))
{
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(granularity must be power of 2));
+goto cleanup;
+}
granularity = param-value.ui;
} else if (STREQ(param-field, VIR_DOMAIN_BLOCK_COPY_BUF_SIZE)) {
buf_size = param-value.ul;
In fact, the virsh man page is not as precise as it could be either:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 63325ff..5d52761 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1039,10 +1039,10 @@ unlimited, but more likely would overflow; it is safer
to use 0 for that
purpose. Specifying Igranularity allows fine-tuning of the granularity that
will be copied when a dirty region is detected; larger values trigger less
I/O overhead but may end up copying more data overall (the default value is
usually correct); {+hypervisors may restrict+} this [-value must-]{+to+} be a
power of [-two.-]{+two or fall+}
{+within a certain range.+} Specifying Ibuf-size will control how much data
can
be simultaneously in-flight during the copy; larger values use more memory but
may allow faster completion (the default value is usually correct).
=item Bblockpull Idomain Ipath [Ibandwidth] [Ibase]
[I--wait [I--verbose] [I--timeout Bseconds] [I--async]]
I'm fixing the man page too, rewording the commit message a bit and ACKing.
Will push after the release.
Oh, thanks a lot for your remind and your fix for man page looks good
for me.
Thanks for your review.
Michal
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH 2/3] libxl: acquire a job when destroying a domain
On Thu, Mar 26, 2015 at 03:29:51PM -0600, Jim Fehlig wrote: Konrad Rzeszutek Wilk wrote: On Wed, Mar 25, 2015 at 02:08:35PM -0600, Jim Fehlig wrote: A job should be acquired at the beginning of a domain destroy operation, not at the end when cleaning up the domain. Fix two occurances of this And s/occurances/occurrences/ here. It looks fine, though, with the squash-in. Also, if you want to have a look at some other things that might be fixed here, plus some speed-up gained, have a look at my commit 540c339a, that does some similar things in the QEMU driver. late job acquisition in the libxl driver. Doing so renders libxlDomainCleanup unused, so it is removed. Just noticed this should be libxlDomainCleanupJob. Yes :) pgp3FiVsl7_Yp.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Bug 1086726: Reworked error messages in libvirt.c, libvirt-domain.c removing uses of __FUNCTION__, except one
---
src/libvirt-domain.c | 13 +++--
src/libvirt.c| 4 ++--
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f1608dc..4a45b9e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -2613,7 +2613,7 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int
flags)
if ((conn-flags VIR_CONNECT_RO)
(flags (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE))) {
virReportError(VIR_ERR_OPERATION_DENIED, %s,
- _(virDomainGetXMLDesc with secure flag));
+ _(Invalid secure flag for XML domain description));
goto error;
}
@@ -2793,7 +2793,7 @@ virDomainMigrateVersion1(virDomainPtr domain,
if (uri == NULL uri_out == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(domainMigratePrepare did not set uri));
+ _(unset uri for domain migration preparation));
goto done;
}
if (uri_out)
@@ -2916,7 +2916,7 @@ virDomainMigrateVersion2(virDomainPtr domain,
if (uri == NULL uri_out == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(domainMigratePrepare2 did not set uri));
+ _(unset uri for domain migration preparation2));
cancelled = 1;
/* Make sure Finish doesn't overwrite the error */
orig_err = virSaveLastError();
@@ -3124,7 +3124,7 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_URI, uri) = 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(domainMigratePrepare3 did not set uri));
+ _(unset uri for domain migration preparation3));
cancelled = 1;
orig_err = virSaveLastError();
goto finish;
@@ -11285,7 +11285,7 @@ virDomainListGetStats(virDomainPtr *doms,
if (!*doms) {
virReportError(VIR_ERR_INVALID_ARG,
- _(doms array in %s must contain at least one domain),
+ _(doms array must contain at least one domain in %s),
__FUNCTION__);
goto cleanup;
}
@@ -11503,7 +11503,8 @@ virDomainInterfaceAddresses(virDomainPtr dom,
return ret;
}
-virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+virReportError(VIR_ERR_NO_SUPPORT, %s,
+ _(No interface address support for domain object));
error:
virDispatchError(dom-conn);
diff --git a/src/libvirt.c b/src/libvirt.c
index c8a5834..0109734 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -558,7 +558,7 @@ virSetSharedInterfaceDriver(virInterfaceDriverPtr driver)
if (virSharedInterfaceDriver) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(A interface driver is already registered));
+ _(An interface driver is already registered));
return -1;
}
@@ -1211,7 +1211,7 @@ do_open(const char *name,
if (!ret-driver) {
/* If we reach here, then all drivers declined the connection. */
-virReportError(VIR_ERR_NO_CONNECT, %s, NULLSTR(name));
+virReportError(VIR_ERR_NO_CONNECT, %s, _(All device drivers decline
connection));
goto failed;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nodeinfodata: Remove broken symlinks
The 7c3c7f217ebae5 commit introduced a nodeinfo test. In order to do that, some parts of sysfs had to be copied. However, sysfs is full of symlinks, so during copying some symlinks broke. Remove them, as on different systems they can point to different files or be broken. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu5/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu6/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu7/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu0/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu0/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu1/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu1/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu2/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu2/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu3/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu3/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu4/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu4/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu5/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu5/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu6/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu6/subsystem | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu7/firmware_node | 1 - tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu7/subsystem | 1 - 24 files changed, 24 deletions(-) delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu5/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu6/subsystem delete mode 12 tests/nodeinfodata/linux-f21-mustang/cpu/cpu7/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu0/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu0/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu1/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu1/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu2/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu2/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu3/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu3/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu4/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu4/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu5/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu5/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu6/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu6/subsystem delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu7/firmware_node delete mode 12 tests/nodeinfodata/linux-rhelsa-3.19.0-mustang/cpu/cpu7/subsystem diff --git a/tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem b/tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem deleted file mode 12 index 758c291..000 --- a/tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem +++ /dev/null @@ -1 +0,0 @@ -../../../../bus/cpu \ No newline at end of file diff --git a/tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/subsystem b/tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/subsystem deleted file mode 12 index 758c291..000 --- a/tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/subsystem +++ /dev/null @@ -1 +0,0 @@ -../../../../bus/cpu \ No newline at end of file diff --git a/tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/subsystem
Re: [libvirt] [PATCHv2] qemu: Fix issues with maxMemory in qemuDomainSetMemoryFlags()
On Mon, Mar 30, 2015 at 09:01:37PM +0200, Peter Krempa wrote:
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.
ACK, since it's a corner-case, after release.
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
pgp9iZz2XiY8i.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] libxl: Move job acquisition in libxlDomainStart to callers
On Wed, Mar 25, 2015 at 02:08:34PM -0600, Jim Fehlig wrote:
Let callers of libxlDomainStart decide when it is appropriate to
acquire a job on the associated virDomainObj.
This makes sense, I see many bugs this fixes, but how come
libxlDomainShutdownThread(), libxlDomainRestoreFlags() and
libxlDoMigrateReceive() don't need to start the job when they all call
libxlDomainStart()?
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/libxl/libxl_domain.c | 24 --
src/libxl/libxl_driver.c | 53 +++-
2 files changed, 52 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 05f6eb1..da4c1c7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -885,17 +893,26 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
goto cleanup;
def = NULL;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+goto cleanup;
+}
+
This should be acquired before virDomainObjListAdd() since you need to
check whether it's active after creating the job. If I'm wrong, then
virDomainObjListRemove() should be only called if the vm is not
persistent (as CreateXML can be called on persistent ones as well),
shouldn't it?
[...]
@@ -1695,11 +1712,20 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char
*from,
def = NULL;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) {
+if (!vm-persistent) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
+goto cleanup;
+}
+
Same here, I guess.
ret = libxlDomainStart(driver, vm, (flags VIR_DOMAIN_SAVE_PAUSED) != 0,
fd);
-if (ret 0 !vm-persistent) {
+if (ret 0 !vm-persistent)
virDomainObjListRemove(driver-domains, vm);
+
+if (!libxlDomainObjEndJob(driver, vm))
vm = NULL;
-}
cleanup:
if (VIR_CLOSE(fd) 0)
@@ -2567,17 +2593,24 @@ libxlDomainCreateWithFlags(virDomainPtr dom,
if (virDomainCreateWithFlagsEnsureACL(dom-conn, vm-def) 0)
goto cleanup;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0)
+goto cleanup;
+
if (virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
%s, _(Domain is already running));
-goto cleanup;
+goto endjob;
}
ret = libxlDomainStart(driver, vm, (flags VIR_DOMAIN_START_PAUSED) != 0,
-1);
if (ret 0)
-goto cleanup;
+goto endjob;
dom-id = vm-def-id;
+ endjob:
+if (!libxlDomainObjEndJob(driver, vm))
+vm = NULL;
+
cleanup:
if (vm)
virObjectUnlock(vm);
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
pgpeaDXGTmqeK.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: lifecycle: make agent-mode shutdown and reboot timeout
When we shutdown/reboot a guest using agent-mode, if the guest itself blocks
infinitely,
libvirt would block in qemuAgentShutdown() forever.
Thus, we set a timeout for shutdown/reboot, from our experience, 60 seconds
would be fine.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
---
include/libvirt/libvirt-qemu.h | 1 +
src/qemu/qemu_agent.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 0c5d650..2bb8ee8 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -49,6 +49,7 @@ typedef enum {
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
} virDomainQemuAgentCommandTimeoutValues;
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index a7b3279..548d580 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1300,7 +1300,7 @@ int qemuAgentShutdown(qemuAgentPtr mon,
else
mon-await_event = QEMU_AGENT_EVENT_SHUTDOWN;
ret = qemuAgentCommand(mon, cmd, reply, false,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN);
virJSONValueFree(cmd);
virJSONValueFree(reply);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/9] qemu: Add quorum support to libvirt
On 31.03.2015 14:39, Matthias Gatto wrote: On Tue, Mar 17, 2015 at 8:25 PM, Matthias Gatto matthias.ga...@outscale.com wrote: The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child. 5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one) V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. V4: -Rebase on master Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name docs/formatdomain.html.in | 27 - docs/schemas/domaincommon.rng | 95 +++-- docs/schemas/storagecommon.rng| 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c| 195 ++ src/conf/storage_conf.c | 22 ++-- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 114 src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 30 +++--- src/qemu/qemu_migration.c | 1 + src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 35 +++--- src/storage/storage_backend_fs.c | 37 --- src/storage/storage_backend_gluster.c | 10 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 116 +--- src/util/virstoragefile.h | 13 ++- tests/virstoragetest.c| 18 ++-- 23 files changed, 573 insertions(+), 175 deletions(-) -- 1.8.3.1 ping This slipped yet another release. Sorry for overlooking this. I'll review this after the release. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] error: negative width in bit-field '_gl_verify_error_if_negative' ?
HI, Does anyone hit this issue when compiling libvirt? CCLD libvirt.la CC libvirt_qemu_la-libvirt-qemu.lo CCLD libvirt-qemu.la CC libvirt_lxc_la-libvirt-lxc.lo CCLD libvirt-lxc.la CC lockd_la-lock_driver_lockd.lo CC lockd_la-lock_protocol.lo CCLD lockd.la CC libvirt_driver_qemu_impl_la-qemu_agent.lo CC libvirt_driver_qemu_impl_la-qemu_capabilities.lo qemu/qemu_capabilities.c:55: error: negative width in bit-field '_gl_verify_error_if_negative' make[3]: *** [libvirt_driver_qemu_impl_la-qemu_capabilities.lo] Error 1 make[3]: Leaving directory `/home/zhiyong.wzy/libvirt-source/src' make[2]: *** [all] Error 2 make[2]: Leaving directory `/home/zhiyong.wzy/libvirt-source/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/zhiyong.wzy/libvirt-source' make: *** [all] Error 2 -- Regards, Zhi Yong Wu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libvirt: virsh: Kill all uses of __FUNCTION__ in error messages
The error output of snapshot-revert should be more friendly. There is no
need to show up virDomainRevertToSnapshot to user. virError already includes
__FUNCTION__ information in a separate member of the struct, so repeating
it in the message is redundant and leads to situations where higher level
code ends up reporting the lower level name We correctly converted the
error output making it more succinct and user-friendly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1086726
---
src/libvirt-domain-snapshot.c | 30 +++
src/libvirt-domain.c | 201 ++
2 files changed, 96 insertions(+), 135 deletions(-)
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 9feb669..ac858ba 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -222,26 +222,20 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)
!(flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
-virReportInvalidArg(flags,
-_(use of 'current' flag in %s requires
- 'redefine' flag),
-__FUNCTION__);
+virReportInvalidArg(flags, %s,
+_(use of 'current' flag in requires 'redefine'
flag));
goto error;
}
if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)
(flags VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
-virReportInvalidArg(flags,
-_('redefine' and 'no metadata' flags in %s are
- mutually exclusive),
-__FUNCTION__);
+virReportInvalidArg(flags, %s,
+_('redefine' and 'no metadata' flags in are
mutually exclusive));
goto error;
}
if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)
(flags VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
-virReportInvalidArg(flags,
-_('redefine' and 'halt' flags in %s are mutually
- exclusive),
-__FUNCTION__);
+virReportInvalidArg(flags, %s,
+_('redefine' and 'halt' flags are mutually
exclusive));
goto error;
}
@@ -1084,10 +1078,8 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if ((flags VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING)
(flags VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-virReportInvalidArg(flags,
-_(running and paused flags in %s are mutually
- exclusive),
-__FUNCTION__);
+virReportInvalidArg(flags, %s,
+_(running and paused flags are mutually
exclusive));
goto error;
}
@@ -1146,10 +1138,8 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
if ((flags VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN)
(flags VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
-virReportInvalidArg(flags,
-_(children and children_only flags in %s are
- mutually exclusive),
-__FUNCTION__);
+virReportInvalidArg(flags, %s,
+_(children and children_only flags are mutually
exclusive));
goto error;
}
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4a45b9e..78b7a93 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -385,9 +385,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const char
*uuidstr)
virCheckNonNullArgGoto(uuidstr, error);
if (virUUIDParse(uuidstr, uuid) 0) {
-virReportInvalidArg(uuidstr,
-_(uuidstr in %s must be a valid UUID),
-__FUNCTION__);
+virReportInvalidArg(uuidstr, %s, _(Invalid UUID));
goto error;
}
@@ -1440,9 +1438,9 @@ virDomainScreenshot(virDomainPtr domain,
virCheckReadOnlyGoto(domain-conn-flags, error);
if (domain-conn != stream-conn) {
-virReportInvalidArg(stream,
-_(stream in %s must match connection of domain
'%s'),
-__FUNCTION__, domain-name);
+virReportInvalidArg(stream, %s,
+_(stream must match connection of domain '%s'),
+domain-name);
goto error;
}
@@ -2179,10 +2177,8 @@ virDomainGetMemoryParameters(virDomainPtr domain,
/* At most one of these two flags should be set. */
if ((flags VIR_DOMAIN_AFFECT_LIVE)
(flags VIR_DOMAIN_AFFECT_CONFIG)) {
-virReportInvalidArg(flags,
-_(flags 'affect live' and 'affect config' in %s
- are mutually exclusive),
-__FUNCTION__);
+
Re: [libvirt] [PATCH] nodeinfodata: Remove broken symlinks
On Wed, Apr 01, 2015 at 11:01:11AM +0200, Michal Privoznik wrote:
The 7c3c7f217ebae5 commit introduced a nodeinfo test. In order to do
that, some parts of sysfs had to be copied. However, sysfs is full of
symlinks, so during copying some symlinks broke. Remove them, as on
different systems they can point to different files or be broken.
That commit introduced only two thirds of the files, the rest is from
f5c2d6 (just in case you want to update the commit message).
But! Not only we don't need these, but power and uevent don't need to
be there either, do they? And core_siblings(_list) is also not
something we (will) use since it's just a syntax sugar for parsing
data for all CPUs (that we do anyway).
If you want to clean the test data a bit, I suggest you clean
tests/nodeinfodata/*/cpu/cpu*/{uevent,power,topology/core_siblings*,firmware_node,subsystem}
files too. That's 202 files you can get rid of.
pgptubFAmTO7W.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Bug 1086726: Reworked error messages in libvirt.c, libvirt-domain.c removing uses of __FUNCTION__, except one
Hello,
We tend to do different formatting of commit messages. If you do 'git log'
you'll immediately get a picture.
I see, forgot to check the previous commits first before doing mine. My
bad, will do better next time.
The idea of the bug report is to do something like this:
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4a45b9e..c39590d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -385,9 +385,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const
char *uuidstr)
virCheckNonNullArgGoto(uuidstr, error);
if (virUUIDParse(uuidstr, uuid) 0) {
-virReportInvalidArg(uuidstr,
-_(uuidstr in %s must be a valid UUID),
-__FUNCTION__);
+virReportInvalidArg(uuidstr, %s, _(invalid UUID));
goto error;
}
@@ -2180,9 +2178,8 @@ virDomainGetMemoryParameters(virDomainPtr domain,
if ((flags VIR_DOMAIN_AFFECT_LIVE)
(flags VIR_DOMAIN_AFFECT_CONFIG)) {
virReportInvalidArg(flags,
-_(flags 'affect live' and 'affect config' in
%s
- are mutually exclusive),
-__FUNCTION__);
+_(flags 'affect live' and 'affect config'
+ are mutually exclusive));
goto error;
}
conn = domain-con
I see. I'll get it then, and re-send.
Thanks,
And so on.
Noella
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Bug 1086726: Reworked error messages in libvirt.c, libvirt-domain.c removing uses of __FUNCTION__, except one
On 01.04.2015 12:13, Noella Ashu wrote:
We tend to do different formatting of commit messages. If you do 'git log'
you'll immediately get a picture.
---
src/libvirt-domain.c | 13 +++--
src/libvirt.c| 4 ++--
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f1608dc..4a45b9e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -2613,7 +2613,7 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int
flags)
if ((conn-flags VIR_CONNECT_RO)
(flags (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE))) {
virReportError(VIR_ERR_OPERATION_DENIED, %s,
- _(virDomainGetXMLDesc with secure flag));
+ _(Invalid secure flag for XML domain description));
This actually was a correct message:
virsh dumpxml --security-info $dom
error: operation forbidden: virDomainGetXMLDesc with secure flag
The idea of the bug report is to do something like this:
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4a45b9e..c39590d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -385,9 +385,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const char
*uuidstr)
virCheckNonNullArgGoto(uuidstr, error);
if (virUUIDParse(uuidstr, uuid) 0) {
-virReportInvalidArg(uuidstr,
-_(uuidstr in %s must be a valid UUID),
-__FUNCTION__);
+virReportInvalidArg(uuidstr, %s, _(invalid UUID));
goto error;
}
@@ -2180,9 +2178,8 @@ virDomainGetMemoryParameters(virDomainPtr domain,
if ((flags VIR_DOMAIN_AFFECT_LIVE)
(flags VIR_DOMAIN_AFFECT_CONFIG)) {
virReportInvalidArg(flags,
-_(flags 'affect live' and 'affect config' in %s
- are mutually exclusive),
-__FUNCTION__);
+_(flags 'affect live' and 'affect config'
+ are mutually exclusive));
goto error;
}
conn = domain-conn;
And so on.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] hostdev: Report the domain name for used hostdevs during nodedev-detach
The nodedev-detach can report the name of the domain using the device
just the way nodedev-reattach does it.
Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com
---
src/util/virhostdev.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 23365a3..55eb9c9 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1519,11 +1519,18 @@ int
virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
virPCIDevicePtr pci)
{
+virPCIDeviceAddressPtr devAddr = NULL;
int ret = -1;
virObjectLock(hostdev_mgr-activePCIHostdevs);
virObjectLock(hostdev_mgr-inactivePCIHostdevs);
+if (!(devAddr = virPCIDeviceGetAddress(pci)))
+goto out;
+
+if (virHostdevIsPCINodeDeviceUsed(devAddr, hostdev_mgr))
+goto out;
+
if (virPCIDeviceDetach(pci, hostdev_mgr-activePCIHostdevs,
hostdev_mgr-inactivePCIHostdevs) 0) {
goto out;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] scsi: Check for invalid target.path after processLU failure
On 03/31/2015 07:57 AM, Ján Tomko wrote:
On Mon, Mar 30, 2015 at 07:16:34PM -0400, John Ferlan wrote:
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)) {
Checking for *found here seems pointless. After the logic change in the
previous patch, it is implied by either of the following conditions:
a) If the target path does not start with /dev, *found will be false:
virStorageBackendStablePath will short-circuit, just strduping
the volume path, and virStorageBackendSCSINewLun will return -1
in that case.
b) If the target path does not exist, it will either be rejected by the
above code path, or by the failed opendir in
virStorageBackendStablePath.
If all you want to do is forbid to start an {,i}SCSI pool that does not
start with /dev, we can do that much earlier in {,I}SCSIStartPool.
The goal was to not start one that has a non-existent pool target.path,
but where/how that's determined is a little bit more involved than other
pools which could use virFileExists() on the pool's target.path in a
Check or Refresh and decide that it's not possible to start the pool.
For iscsi, that path creation is not managed by libvirt, hence the
wait loop in virStorageBackendStablePath.
I suppose I could check in start/check that if the target.path doesn't
start with /dev[/], then do a virFileExists on the provided path. If
that path doesn't exist, then fail the startup. That would solve the
bug without messing with processLU's return values.
To catch wrong paths in /dev, I think the proper way is to stop ignoring
the return value of processLU and make it return -1 on fatal errors
(OOM, pool target path does not exist, etc.) and -2 on errors that won't
necessarily stop us from finding other volumes.
Having processLU return 1 had more to do with distinguishing the
difference between a non disk/lun and a finding a disk/lun. What I found
was for iSCSI found = true was being set because of the
/sys/bus/scsi/devices/id/type file had 0xC (or 12 or a controller)
(id is the host:bus:target:lun).
The concern over wrong paths or something that doesn't start with
/dev[/] and having it be a failure is there has to be a reason a non
/dev[/] path was allowed and if we return -1 just because of that I'm
unclear what effect that has since the code is shared between scsi and
iscsi... I do agree that other failures in virStorageBackendSCSINewLun
should be differentiated.
I've made some adjustments and will repost shortly
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nodeinfodata: Remove broken symlinks
On 01.04.2015 15:02, Martin Kletzander wrote:
On Wed, Apr 01, 2015 at 11:01:11AM +0200, Michal Privoznik wrote:
The 7c3c7f217ebae5 commit introduced a nodeinfo test. In order to do
that, some parts of sysfs had to be copied. However, sysfs is full of
symlinks, so during copying some symlinks broke. Remove them, as on
different systems they can point to different files or be broken.
That commit introduced only two thirds of the files, the rest is from
f5c2d6 (just in case you want to update the commit message).
But! Not only we don't need these, but power and uevent don't need to
be there either, do they? And core_siblings(_list) is also not
something we (will) use since it's just a syntax sugar for parsing
data for all CPUs (that we do anyway).
If you want to clean the test data a bit, I suggest you clean
tests/nodeinfodata/*/cpu/cpu*/{uevent,power,topology/core_siblings*,firmware_node,subsystem}
files too. That's 202 files you can get rid of.
good point. Although I'd leave core_siblings and topology - it's
currently not used, but it may come handy when debugging our own node
info code. It contains all the pieces of information that one need to
reconstruct the topology. But others can be removed as you say. Will
send v2.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nodeinfodata: Remove broken symlinks
On Wed, Apr 01, 2015 at 03:30:50PM +0200, Michal Privoznik wrote:
On 01.04.2015 15:02, Martin Kletzander wrote:
On Wed, Apr 01, 2015 at 11:01:11AM +0200, Michal Privoznik wrote:
The 7c3c7f217ebae5 commit introduced a nodeinfo test. In order to do
that, some parts of sysfs had to be copied. However, sysfs is full of
symlinks, so during copying some symlinks broke. Remove them, as on
different systems they can point to different files or be broken.
That commit introduced only two thirds of the files, the rest is from
f5c2d6 (just in case you want to update the commit message).
But! Not only we don't need these, but power and uevent don't need to
be there either, do they? And core_siblings(_list) is also not
something we (will) use since it's just a syntax sugar for parsing
data for all CPUs (that we do anyway).
If you want to clean the test data a bit, I suggest you clean
tests/nodeinfodata/*/cpu/cpu*/{uevent,power,topology/core_siblings*,firmware_node,subsystem}
files too. That's 202 files you can get rid of.
good point. Although I'd leave core_siblings and topology - it's
currently not used, but it may come handy when debugging our own node
info code. It contains all the pieces of information that one need to
reconstruct the topology. But others can be removed as you say. Will
send v2.
I meant that the info in core_siblings* can be reconstructed from all
the other data, but that's right that we might validate it with that
additional files.
pgpBpMY4nxopJ.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] nodeinfodata: Remove broken symlinks and uneeded files
The 7c3c7f217ebae5 and f5c2d6 commits introduced a nodeinfo test. In order to do that, some parts of sysfs had to be copied. However, sysfs is full of symlinks, so during copying some symlinks broke. Remove them, as on different systems they can point to different files or be broken. At the same time, we don't need all files added in those commits. For instance we don't care about 'uevent' files, 'power' folders, and others. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v1: - remove more files tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/power/async | 1 - .../linux-f21-mustang/cpu/cpu0/power/autosuspend_delay_ms | 0 tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/power/control | 1 - .../linux-f21-mustang/cpu/cpu0/power/runtime_active_kids | 1 - .../linux-f21-mustang/cpu/cpu0/power/runtime_active_time | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu0/power/runtime_enabled | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu0/power/runtime_status | 1 - .../linux-f21-mustang/cpu/cpu0/power/runtime_suspended_time | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/power/runtime_usage | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu0/uevent | 8 tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/power/async | 1 - .../linux-f21-mustang/cpu/cpu1/power/autosuspend_delay_ms | 0 tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/power/control | 1 - .../linux-f21-mustang/cpu/cpu1/power/runtime_active_kids | 1 - .../linux-f21-mustang/cpu/cpu1/power/runtime_active_time | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu1/power/runtime_enabled | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu1/power/runtime_status | 1 - .../linux-f21-mustang/cpu/cpu1/power/runtime_suspended_time | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/power/runtime_usage | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu1/uevent | 8 tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/power/async | 1 - .../linux-f21-mustang/cpu/cpu2/power/autosuspend_delay_ms | 0 tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/power/control | 1 - .../linux-f21-mustang/cpu/cpu2/power/runtime_active_kids | 1 - .../linux-f21-mustang/cpu/cpu2/power/runtime_active_time | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu2/power/runtime_enabled | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu2/power/runtime_status | 1 - .../linux-f21-mustang/cpu/cpu2/power/runtime_suspended_time | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/power/runtime_usage | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu2/uevent | 8 tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/power/async | 1 - .../linux-f21-mustang/cpu/cpu3/power/autosuspend_delay_ms | 0 tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/power/control | 1 - .../linux-f21-mustang/cpu/cpu3/power/runtime_active_kids | 1 - .../linux-f21-mustang/cpu/cpu3/power/runtime_active_time | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu3/power/runtime_enabled | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu3/power/runtime_status | 1 - .../linux-f21-mustang/cpu/cpu3/power/runtime_suspended_time | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/power/runtime_usage | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu3/uevent | 8 tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/power/async | 1 - .../linux-f21-mustang/cpu/cpu4/power/autosuspend_delay_ms | 0 tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/power/control | 1 - .../linux-f21-mustang/cpu/cpu4/power/runtime_active_kids | 1 - .../linux-f21-mustang/cpu/cpu4/power/runtime_active_time | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu4/power/runtime_enabled | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu4/power/runtime_status | 1 - .../linux-f21-mustang/cpu/cpu4/power/runtime_suspended_time | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/power/runtime_usage | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/subsystem | 1 - tests/nodeinfodata/linux-f21-mustang/cpu/cpu4/uevent | 8 tests/nodeinfodata/linux-f21-mustang/cpu/cpu5/power/async | 1 - .../linux-f21-mustang/cpu/cpu5/power/autosuspend_delay_ms | 0 tests/nodeinfodata/linux-f21-mustang/cpu/cpu5/power/control | 1 - .../linux-f21-mustang/cpu/cpu5/power/runtime_active_kids | 1 - .../linux-f21-mustang/cpu/cpu5/power/runtime_active_time | 1 - .../nodeinfodata/linux-f21-mustang/cpu/cpu5/power/runtime_enabled | 1 -
Re: [libvirt] [PATCH v2] nodeinfodata: Remove broken symlinks and uneeded files
On 04/01/2015 10:12 AM, Michal Privoznik wrote: The 7c3c7f217ebae5 and f5c2d6 commits introduced a nodeinfo test. In order to do that, some parts of sysfs had to be copied. However, sysfs is full of symlinks, so during copying some symlinks broke. Remove them, as on different systems they can point to different files or be broken. At the same time, we don't need all files added in those commits. For instance we don't care about 'uevent' files, 'power' folders, and others. Signed-off-by: Michal Privoznik mpriv...@redhat.com FWIW I just tar'd up the directory. Even if we don't use some of the files now, IMO it's safer long term to import the dir rather than cherry-pick the files libvirt currently uses (the broken symlinks are useless though). But it's not a big deal to me so I'll leave it up to you guys - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nodeinfodata: Remove broken symlinks and uneeded files
On Wed, Apr 01, 2015 at 04:12:18PM +0200, Michal Privoznik wrote: The 7c3c7f217ebae5 and f5c2d6 commits introduced a nodeinfo test. In order to do that, some parts of sysfs had to be copied. However, sysfs is full of symlinks, so during copying some symlinks broke. Remove them, as on different systems they can point to different files or be broken. At the same time, we don't need all files added in those commits. For instance we don't care about 'uevent' files, 'power' folders, and others. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v1: - remove more files s/needed/unneeded/ in $subj ACK after release with that changed. pgpJQY2O0q8bM.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] Use the DEFAULT_DRIVER_DIR macro
Unused since commit bc2f42a0. Move it under the WITHOUT_DRIVER_MODULES #ifdef and start using it again. --- src/driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/driver.c b/src/driver.c index db03438..2985538 100644 --- a/src/driver.c +++ b/src/driver.c @@ -35,13 +35,13 @@ VIR_LOG_INIT(driver); -#define DEFAULT_DRIVER_DIR LIBDIR /libvirt/connection-driver #ifdef WITH_DRIVER_MODULES /* XXX re-implment this for other OS, or use libtools helper lib ? */ # include dlfcn.h +# define DEFAULT_DRIVER_DIR LIBDIR /libvirt/connection-driver void * virDriverLoadModule(const char *name) @@ -57,7 +57,7 @@ virDriverLoadModule(const char *name) libvirt_driver_, .so, abs_topbuilddir /src/.libs, -LIBDIR /libvirt/connection-driver, +DEFAULT_DRIVER_DIR, LIBVIRT_DRIVER_DIR))) return NULL; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Cleanups
First two patches are a result of compiling with -Wunused-macros. They reduce the number of infractions from 111 to 82. The remaining cases are: * unused VIR_FROM_THIS * copies of other headers * in drivers I'm not familiar with enough The next two were found randomly, which lead me to look into virutil.h as well, because just like virsh.c, the util files were split as well. Ján Tomko (6): Remove unused macros Use the DEFAULT_DRIVER_DIR macro Do not include cpu_map.h in libvirtd.c Clean up headers in src/util/virutil.h Remove unused includes from virsh Remove unnecessary includes from virsh.h daemon/libvirtd.c | 2 -- src/conf/network_conf.c | 1 - src/driver.c| 4 ++-- src/locking/lock_daemon.c | 1 - src/locking/lock_driver_lockd.c | 4 src/openvz/openvz_driver.c | 4 src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor_text.c| 3 --- src/test/test_driver.c | 1 - src/uml/uml_driver.c| 3 --- src/util/virutil.c | 4 src/util/virutil.h | 2 -- tests/sockettest.c | 10 -- tests/testutils.c | 5 - tools/virsh-domain.c| 1 + tools/virsh-network.c | 7 +-- tools/virsh-nodedev.c | 7 +-- tools/virsh-nwfilter.c | 6 -- tools/virsh-pool.c | 6 -- tools/virsh-secret.c| 6 -- tools/virsh-snapshot.c | 1 + tools/virsh.c | 10 -- tools/virsh.h | 3 --- 23 files changed, 6 insertions(+), 87 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] Remove unused includes from virsh
After splitting out most of virsh command, some includes are no longer needed. Some files have the libXML includes despite not needing them. --- tools/virsh-network.c | 6 -- tools/virsh-nodedev.c | 6 -- tools/virsh-nwfilter.c | 6 -- tools/virsh-pool.c | 6 -- tools/virsh-secret.c | 6 -- tools/virsh.c | 10 -- tools/virsh.h | 1 - 7 files changed, 41 deletions(-) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 62323c4..39f0266 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -26,16 +26,10 @@ #include config.h #include virsh-network.h -#include libxml/parser.h -#include libxml/tree.h -#include libxml/xpath.h -#include libxml/xmlsave.h - #include internal.h #include virbuffer.h #include viralloc.h #include virfile.h -#include virxml.h #include conf/network_conf.h virNetworkPtr diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 46e0045..847e5b0 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -26,16 +26,10 @@ #include config.h #include virsh-nodedev.h -#include libxml/parser.h -#include libxml/tree.h -#include libxml/xpath.h -#include libxml/xmlsave.h - #include internal.h #include virbuffer.h #include viralloc.h #include virfile.h -#include virxml.h #include conf/node_device_conf.h /* diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 2a75175..e6bef08 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -26,17 +26,11 @@ #include config.h #include virsh-nwfilter.h -#include libxml/parser.h -#include libxml/tree.h -#include libxml/xpath.h -#include libxml/xmlsave.h - #include internal.h #include virbuffer.h #include viralloc.h #include virfile.h #include virutil.h -#include virxml.h virNWFilterPtr vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 485d23d..4865831 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -26,16 +26,10 @@ #include config.h #include virsh-pool.h -#include libxml/parser.h -#include libxml/tree.h -#include libxml/xpath.h -#include libxml/xmlsave.h - #include internal.h #include virbuffer.h #include viralloc.h #include virfile.h -#include virxml.h #include conf/storage_conf.h #include virstring.h diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index add2c09..5065c6f 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -26,18 +26,12 @@ #include config.h #include virsh-secret.h -#include libxml/parser.h -#include libxml/tree.h -#include libxml/xpath.h -#include libxml/xmlsave.h - #include internal.h #include base64.h #include virbuffer.h #include viralloc.h #include virfile.h #include virutil.h -#include virxml.h #include conf/secret_conf.h static virSecretPtr diff --git a/tools/virsh.c b/tools/virsh.c index 9ecddf3..889a561 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -44,11 +44,6 @@ #include strings.h #include signal.h -#include libxml/parser.h -#include libxml/tree.h -#include libxml/xpath.h -#include libxml/xmlsave.h - #if WITH_READLINE # include readline/readline.h # include readline/history.h @@ -56,19 +51,14 @@ #include internal.h #include virerror.h -#include base64.h #include virbuffer.h #include viralloc.h -#include virxml.h #include libvirt/libvirt-qemu.h #include libvirt/libvirt-lxc.h #include virfile.h #include configmake.h #include virthread.h #include vircommand.h -#include virkeycode.h -#include virnetdevbandwidth.h -#include virbitmap.h #include conf/domain_conf.h #include virtypedparam.h #include virstring.h diff --git a/tools/virsh.h b/tools/virsh.h index df2ea7f..3c28454 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -31,7 +31,6 @@ # include stdarg.h # include unistd.h # include sys/stat.h -# include inttypes.h # include termios.h # include internal.h -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] Remove unnecessary includes from virsh.h
Include them in the files that need them instead. --- tools/virsh-domain.c | 1 + tools/virsh-network.c | 1 + tools/virsh-nodedev.c | 1 + tools/virsh-snapshot.c | 1 + tools/virsh.h | 2 -- 5 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9bbb964..eee441f 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -47,6 +47,7 @@ #include virjson.h #include virkeycode.h #include virmacaddr.h +#include virnetdevbandwidth.h #include virprocess.h #include virstring.h #include virsh-console.h diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 39f0266..09ee11a 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -30,6 +30,7 @@ #include virbuffer.h #include viralloc.h #include virfile.h +#include virstring.h #include conf/network_conf.h virNetworkPtr diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 847e5b0..0d7315c 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -30,6 +30,7 @@ #include virbuffer.h #include viralloc.h #include virfile.h +#include virstring.h #include conf/node_device_conf.h /* diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 1bb74a6..459a8a8 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -38,6 +38,7 @@ #include viralloc.h #include virfile.h #include virsh-domain.h +#include virstring.h #include virxml.h #include conf/snapshot_conf.h diff --git a/tools/virsh.h b/tools/virsh.h index 3c28454..e89d315 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -36,8 +36,6 @@ # include internal.h # include virerror.h # include virthread.h -# include virnetdevbandwidth.h -# include virstring.h # define VSH_MAX_XML_FILE (10*1024*1024) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] Remove unused macros
In the order of appearance:
* MAX_LISTEN - never used
added by 23ad665c (qemud) and addec57 (lock daemon)
* NEXT_FREE_CLASS_ID - never used, added by 07d1b6b
* virLockError - never used, added by eb8268a4
* OPENVZ_MAX_ARG, CMDBUF_LEN, CMDOP_LEN
unused since the removal of ADD_ARG_LIT in d8b31306
* QEMU_NB_PER_CPU_STAT_PARAM - unused since 897808e
* QEMU_CMD_PROMPT, QEMU_PASSWD_PROMPT - unused since 1dc10a7
* TEST_MODEL_WORDSIZE - unused since c25c18f7
* TEMPDIR - never used, added by 714bef5
* NSIG - workaround around old headers
added by commit 60ed1d2
unused since virExec was moved by commit 02e8691
* DO_TEST_PARSE - never used, added by 9afa006
* DIFF_MSEC, GETTIMEOFDAY - unused since eee6eb6
---
daemon/libvirtd.c | 1 -
src/conf/network_conf.c | 1 -
src/locking/lock_daemon.c | 1 -
src/locking/lock_driver_lockd.c | 4
src/openvz/openvz_driver.c | 4
src/qemu/qemu_driver.c | 2 --
src/qemu/qemu_monitor_text.c| 3 ---
src/test/test_driver.c | 1 -
src/uml/uml_driver.c| 3 ---
src/util/virutil.c | 4
tests/sockettest.c | 10 --
tests/testutils.c | 5 -
12 files changed, 39 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 2366d63..55acee2 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1097,7 +1097,6 @@ daemonUsage(const char *argv0, bool privileged)
}
}
-#define MAX_LISTEN 5
int main(int argc, char **argv) {
virNetServerPtr srv = NULL;
char *remote_config_file = NULL;
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b334b64..f4a9df0 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -47,7 +47,6 @@
#define MAX_BRIDGE_ID 256
#define VIR_FROM_THIS VIR_FROM_NETWORK
-#define NEXT_FREE_CLASS_ID 3
/* currently, /sbin/tc implementation allows up to 16 bits for minor class
size */
#define CLASS_ID_BITMAP_SIZE (116)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 8cc6979..bb165c0 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -,7 +,6 @@ virLockDaemonUsage(const char *argv0, bool privileged)
}
}
-#define MAX_LISTEN 5
int main(int argc, char **argv) {
virNetServerProgramPtr lockProgram = NULL;
char *remote_config_file = NULL;
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 72a4a0c..3a48a6a 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -38,10 +38,6 @@
VIR_LOG_INIT(locking.lock_driver_lockd);
-#define virLockError(code, ...) \
-virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
- __FUNCTION__, __LINE__, __VA_ARGS__)
-
typedef struct _virLockManagerLockDaemonPrivate
virLockManagerLockDaemonPrivate;
typedef virLockManagerLockDaemonPrivate *virLockManagerLockDaemonPrivatePtr;
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index d29e35b..f9f924f 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -63,10 +63,6 @@
VIR_LOG_INIT(openvz.openvz_driver);
-#define OPENVZ_MAX_ARG 28
-#define CMDBUF_LEN 1488
-#define CMDOP_LEN 288
-
#define OPENVZ_NB_MEM_PARAM 3
static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index becf415..9e28bf9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -110,8 +110,6 @@ VIR_LOG_INIT(qemu.qemu_driver);
#define QEMU_NB_NUMA_PARAM 2
-#define QEMU_NB_PER_CPU_STAT_PARAM 2
-
#define QEMU_SCHED_MIN_PERIOD 1000LL
#define QEMU_SCHED_MAX_PERIOD 100LL
#define QEMU_SCHED_MIN_QUOTA 1000LL
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 9973a17..f26bc78 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -51,9 +51,6 @@
VIR_LOG_INIT(qemu.qemu_monitor_text);
-#define QEMU_CMD_PROMPT \n(qemu)
-#define QEMU_PASSWD_PROMPT Password:
-
#define DEBUG_IO 0
/* Return -1 for error, 0 for success */
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 07cc032..d49c4b3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -122,7 +122,6 @@ static int defaultConnections;
static virMutex defaultLock = VIR_MUTEX_INITIALIZER;
#define TEST_MODEL i686
-#define TEST_MODEL_WORDSIZE 32
#define TEST_EMULATOR /usr/bin/test-hv
static const virNodeInfo defaultNodeInfo = {
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 2d59126..6b4f655 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -71,9 +71,6 @@
VIR_LOG_INIT(uml.uml_driver);
-/* For storing short-lived temporary files. */
-#define TEMPDIR LOCALSTATEDIR /cache/libvirt
-
typedef struct _umlDomainObjPrivate umlDomainObjPrivate;
typedef
[libvirt] [PATCH 4/6] Clean up headers in src/util/virutil.h
* verify.h from gnulib is included in internal.h * sys/select.h is no longer needed added by commit da196338 to use fd_set in virExec prototype --- src/util/virutil.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/util/virutil.h b/src/util/virutil.h index 25524e2..55a3bd6 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -25,10 +25,8 @@ #ifndef __VIR_UTIL_H__ # define __VIR_UTIL_H__ -# include verify.h # include internal.h # include unistd.h -# include sys/select.h # include sys/types.h # ifndef MIN -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] Do not include cpu_map.h in libvirtd.c
No longer needed after commit dd47723 --- daemon/libvirtd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 55acee2..107b88d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -105,7 +105,6 @@ #include configmake.h #include virdbus.h -#include cpu/cpu_map.h VIR_LOG_INIT(daemon.libvirtd); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] hostdev: fix net config restore error
On 2015年03月31日 17:46, Martin Kletzander wrote:
On Thu, Mar 26, 2015 at 10:23:47PM +0800, Huanle Han wrote:
Fix for such a case:
1. Domain A and B xml contain the same SRIOV net hostdev(interface
type='hostdev' / with same pci address).
2. virsh start A (Successfully, and configure the SRIOV net with
custom mac)
3. virsh start B (Fail because of the hostdev used by domain A or other
reason.)
In step 3, 'virHostdevNetConfigRestore' is called for the hostdev
which is still used by domain A. It makes the mac/vlan of the SRIOV net
change.
Makes perfect sense, thanks for finding that out.
Code Change in this fix:
1. As the pci used by other domain have been removed from
'pcidevs' in previous loop, we only restore the nic config for
the hostdev still in 'pcidevs'(used by this domain)
2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the
hostdev is a pci net device or not.
3. update the comments to make it more clear
Signed-off-by: Huanle Han hanxue...@gmail.com hanxue...@gmail.com
---
src/util/virhostdev.c | 52
---
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 83f567d..b04bae2 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -350,6 +350,14 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev,
char **linkdev,
return ret;
}
+static int
+virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev)
+{
+return hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS
+hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
+hostdev-parent.type == VIR_DOMAIN_DEVICE_NET
+hostdev-parent.data.net;
+}
static int
virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf,
@@ -481,10 +489,7 @@ virHostdevNetConfigRestore(virDomainHostdevDefPtr
hostdev,
/* This is only needed for PCI devices that have been defined
* using interface type='hostdev'. For all others, it is a NOP.
*/
-if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
-hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
||
-hostdev-parent.type != VIR_DOMAIN_DEVICE_NET ||
-!hostdev-parent.data.net)
+if (!virHostdevIsPCINetDevice(hostdev))
return 0;
isvf = virHostdevIsVirtualFunction(hostdev);
@@ -781,19 +786,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr
hostdev_mgr,
goto cleanup;
}
-/* Again 4 loops; mark all devices as inactive before reset
+/* Here are 4 loops; mark all devices as inactive before reset
* them and reset all the devices before re-attach.
* Attach mac and port profile parameters to devices
*/
+
+/* Loop 1: delete the copy of the dev from pcidevs if it's used by
+ * other domain. Or delete it from activePCIHostDevs if it had
+ * been used by this domain.
+ */
i = 0;
while (i virPCIDeviceListCount(pcidevs)) {
virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
virPCIDevicePtr activeDev = NULL;
-/* delete the copy of the dev from pcidevs if it's used by
- * other domain. Or delete it from activePCIHostDevs if it had
- * been used by this domain.
- */
activeDev = virPCIDeviceListFind(hostdev_mgr-activePCIHostdevs,
dev);
if (activeDev) {
const char *usedby_drvname;
@@ -814,14 +820,27 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr
hostdev_mgr,
* pcidevs, but has been removed from activePCIHostdevs.
*/
-/*
- * For SRIOV net host devices, unset mac and port profile before
- * reset and reattach device
+/* Loop 2: before reset and reattach device,
+ * unset mac and port profile for SRIOV net host devices used by this
domain
*/
The comments were formatted as a sentence, it would be nice to keep it
that way.
Is comment as below OK?
/*
* Loop 2: For SRIOV net host devices used by this domain, unset mac
* and port profile before reset and reattach device
*/
-for (i = 0; i nhostdevs; i++)
-virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr-stateDir,
- oldStateDir);
+for (i = 0; i nhostdevs; i++) {
+virDomainHostdevDefPtr hostdev = hostdevs[i];
+if (virHostdevIsPCINetDevice(hostdev)) {
+virDomainHostdevSubsysPCIPtr pcisrc =
hostdev-source.subsys.u.pci;
+virPCIDevicePtr dev = NULL;
+dev = virPCIDeviceNew(pcisrc-addr.domain, pcisrc-addr.bus,
+ pcisrc-addr.slot,
pcisrc-addr.function);
+
The daemon will crash if this function returns NULL. There should be
check for that, but it shouldn't be fatal, so we can clean up as much
as possible (see Loop 3 for example on how to handle that).
Is this OK?
if (dev) {
if (virPCIDeviceListFind(pcidevs, dev)) {
virHostdevNetConfigRestore(hostdev,
hostdev_mgr-stateDir,
Re: [libvirt] error: negative width in bit-field '_gl_verify_error_if_negative' ?
On 04/01/2015 03:38 AM, Zhi Yong Wu wrote: HI, Does anyone hit this issue when compiling libvirt? CCLD libvirt.la CC libvirt_qemu_la-libvirt-qemu.lo CCLD libvirt-qemu.la CC libvirt_lxc_la-libvirt-lxc.lo CCLD libvirt-lxc.la CC lockd_la-lock_driver_lockd.lo CC lockd_la-lock_protocol.lo CCLD lockd.la CC libvirt_driver_qemu_impl_la-qemu_agent.lo CC libvirt_driver_qemu_impl_la-qemu_capabilities.lo qemu/qemu_capabilities.c:55: error: negative width in bit-field '_gl_verify_error_if_negative' Usually, this means you changed something in code that requires a parallel change somewhere else to keep array sizes in sync, or whatever else the compile-time assertion is checking for. This particular error is part of VIR_ENUM_IMPL, which means you probably added a capability in qemu_capabilities.h but forgot to associate it with a string here. -- 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] error: negative width in bit-field '_gl_verify_error_if_negative' ?
got fixed, thanks. On Thu, Apr 2, 2015 at 12:16 AM, Eric Blake ebl...@redhat.com wrote: On 04/01/2015 03:38 AM, Zhi Yong Wu wrote: HI, Does anyone hit this issue when compiling libvirt? CCLD libvirt.la CC libvirt_qemu_la-libvirt-qemu.lo CCLD libvirt-qemu.la CC libvirt_lxc_la-libvirt-lxc.lo CCLD libvirt-lxc.la CC lockd_la-lock_driver_lockd.lo CC lockd_la-lock_protocol.lo CCLD lockd.la CC libvirt_driver_qemu_impl_la-qemu_agent.lo CC libvirt_driver_qemu_impl_la-qemu_capabilities.lo qemu/qemu_capabilities.c:55: error: negative width in bit-field '_gl_verify_error_if_negative' Usually, this means you changed something in code that requires a parallel change somewhere else to keep array sizes in sync, or whatever else the compile-time assertion is checking for. This particular error is part of VIR_ENUM_IMPL, which means you probably added a capability in qemu_capabilities.h but forgot to associate it with a string here. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- Regards, Zhi Yong Wu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/11] qemu: blockPull: Refactor the rest of qemuDomainBlockJobImpl
Since it now handles only block pull code paths we can refactor it and
remove tons of cruft.
---
src/qemu/qemu_driver.c | 86
src/qemu/qemu_monitor.c | 30
src/qemu/qemu_monitor.h | 17 -
src/qemu/qemu_monitor_json.c | 59 +++---
src/qemu/qemu_monitor_json.h | 13 ---
5 files changed, 78 insertions(+), 127 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7b7775d..2dd8ed4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16162,17 +16162,16 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
/* bandwidth in MiB/s per public API. Caller must lock vm beforehand,
* and not access it afterwards. */
static int
-qemuDomainBlockJobImpl(virDomainObjPtr vm,
- virConnectPtr conn,
- const char *path, const char *base,
- unsigned long bandwidth,
- int mode, unsigned int flags)
+qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *path,
+ const char *base,
+ unsigned long bandwidth,
+ unsigned int flags)
{
-virQEMUDriverPtr driver = conn-privateData;
-qemuDomainObjPrivatePtr priv;
+qemuDomainObjPrivatePtr priv = vm-privateData;
char *device = NULL;
-int ret = -1;
-bool async = false;
+bool modern;
int idx;
virDomainDiskDefPtr disk;
virStorageSourcePtr baseSource = NULL;
@@ -16180,12 +16179,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
char *basePath = NULL;
char *backingPath = NULL;
unsigned long long speed = bandwidth;
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain is not running));
-goto cleanup;
-}
+int ret = -1;
if (flags VIR_DOMAIN_BLOCK_REBASE_RELATIVE !base) {
virReportError(VIR_ERR_INVALID_ARG, %s,
@@ -16194,23 +16188,23 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
goto cleanup;
}
-priv = vm-privateData;
-if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
-async = true;
-} else if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(block jobs not supported with this QEMU binary));
-goto cleanup;
-} else if (base) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(partial block pull not supported with this
- QEMU binary));
-goto cleanup;
-} else if (mode == BLOCK_JOB_PULL bandwidth) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(setting bandwidth at start of block pull not
- supported with this QEMU binary));
+if (qemuDomainSupportsBlockJobs(vm, modern) 0)
goto cleanup;
+
+if (!modern) {
+if (base) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(partial block pull not supported with this
+ QEMU binary));
+goto cleanup;
+}
+
+if (bandwidth) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(setting bandwidth at start of block pull not
+ supported with this QEMU binary));
+goto cleanup;
+}
}
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
@@ -16222,12 +16216,11 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
goto endjob;
}
-device = qemuDiskPathToAlias(vm, path, idx);
-if (!device)
+if (!(device = qemuDiskPathToAlias(vm, path, idx)))
goto endjob;
disk = vm-def-disks[idx];
-if (mode == BLOCK_JOB_PULL qemuDomainDiskBlockJobIsActive(disk))
+if (qemuDomainDiskBlockJobIsActive(disk))
goto endjob;
if (base
@@ -16250,7 +16243,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
backingPath) 0)
goto endjob;
-
if (!backingPath) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(can't keep relative backing relationship));
@@ -16260,8 +16252,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
/* Convert bandwidth MiB to bytes, if needed */
-if (mode == BLOCK_JOB_PULL
-!(flags VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES)) {
+if (!(flags VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES)) {
if (speed LLONG_MAX 20) {
virReportError(VIR_ERR_OVERFLOW,
_(bandwidth must be less than %llu),
@@ -16276,15 +16267,15 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
basePath =
[libvirt] [PATCH 07/11] qemu: blockPivot: Don't pause the VM any more since we don't use drive-reopen
Support for drive-reopen was never present in the upstream code so we
don't need to pause the VM when doing the block pivot. Kill all the
code related to this semi-upstream artifact.
---
src/qemu/qemu_driver.c | 48 +---
1 file changed, 5 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7c33ca3..44ee04f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16043,14 +16043,14 @@ qemuDiskPathToAlias(virDomainObjPtr vm, const char
*path, int *idxret)
* abort with pivot; this updates the VM definition as appropriate, on
* either success or failure. */
static int
-qemuDomainBlockPivot(virConnectPtr conn,
- virQEMUDriverPtr driver, virDomainObjPtr vm,
- const char *device, virDomainDiskDefPtr disk)
+qemuDomainBlockPivot(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *device,
+ virDomainDiskDefPtr disk)
{
int ret = -1, rc;
qemuDomainObjPrivatePtr priv = vm-privateData;
virDomainBlockJobInfo info;
-bool resume = false;
virStorageSourcePtr oldsrc = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -16081,29 +16081,6 @@ qemuDomainBlockPivot(virConnectPtr conn,
goto cleanup;
}
-/* If we are using the older 'drive-reopen', we want to make sure
- * that management apps can tell whether the command succeeded,
- * even if libvirtd is restarted at the wrong time. To accomplish
- * that, we pause the guest before drive-reopen, and resume it
- * only when we know the outcome; if libvirtd restarts, then
- * management will see the guest still paused, and know that no
- * guest I/O has caused the source and mirror to diverge. XXX
- * With the newer 'block-job-complete', we need to use a
- * persistent bitmap to make things safe; so for now, we just
- * blindly pause the guest. */
-if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
-if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE,
-QEMU_ASYNC_JOB_NONE) 0)
-goto cleanup;
-
-resume = true;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(guest unexpectedly quit));
-goto cleanup;
-}
-}
-
/* For active commit, the mirror is part of the already labeled
* chain. For blockcopy, we previously labeled only the top-level
* image; but if the user is reusing an external image that
@@ -16177,21 +16154,6 @@ qemuDomainBlockPivot(virConnectPtr conn,
if (oldsrc)
disk-src = oldsrc;
-if (resume virDomainObjIsActive(vm)
-qemuProcessStartCPUs(driver, vm, conn,
- VIR_DOMAIN_RUNNING_UNPAUSED,
- QEMU_ASYNC_JOB_NONE) 0) {
-virObjectEventPtr event = NULL;
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
-if (event)
-qemuDomainEventQueue(driver, event);
-if (virGetLastError() == NULL) {
-virReportError(VIR_ERR_OPERATION_FAILED, %s,
- _(resuming after drive-reopen failed));
-}
-}
virObjectUnref(cfg);
return ret;
}
@@ -16295,7 +16257,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
if (disk-mirror (flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
-ret = qemuDomainBlockPivot(conn, driver, vm, device, disk);
+ret = qemuDomainBlockPivot(driver, vm, device, disk);
if (ret 0 async)
goto endjob;
goto waitjob;
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/11] qemu: Refactor qemuDomainBlockJobAbort()
Change few variable names and refactor the code flow. As an additional
bonus the function now fails if the event state is not as expected.
---
src/qemu/qemu_driver.c | 108 -
1 file changed, 52 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9431ad..ee5bf8b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16279,13 +16279,13 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
{
virQEMUDriverPtr driver = dom-conn-privateData;
char *device = NULL;
-virObjectEventPtr event = NULL;
-virObjectEventPtr event2 = NULL;
virDomainDiskDefPtr disk;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
bool save = false;
int idx;
-bool async;
+bool modern;
+bool pivot = !!(flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT);
+bool async = !!(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC);
virDomainObjPtr vm;
int ret = -1;
@@ -16298,7 +16298,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
if (virDomainBlockJobAbortEnsureACL(dom-conn, vm-def) 0)
goto cleanup;
-if (qemuDomainSupportsBlockJobs(vm, async) 0)
+if (qemuDomainSupportsBlockJobs(vm, modern) 0)
goto cleanup;
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
@@ -16314,19 +16314,14 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
disk = vm-def-disks[idx];
-if (async !(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
-/* prepare state for event delivery */
+if (modern !async) {
+/* prepare state for event delivery. Since qemuDomainBlockPivot is
+ * synchronous, but the event is delivered asynchronously we need to
+ * wait too */
disk-blockJobStatus = -1;
disk-blockJobSync = true;
}
-if ((flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)
-!(async disk-mirror)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- _(pivot of disk '%s' requires an active copy job),
- disk-dst);
-goto endjob;
-}
if (disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE
disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -16335,29 +16330,29 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
}
-if (disk-mirror (flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
-ret = qemuDomainBlockPivot(driver, vm, device, disk);
-if (ret 0 async)
+if (pivot) {
+if (qemuDomainBlockPivot(driver, vm, device, disk) 0)
goto endjob;
-goto waitjob;
-}
-if (disk-mirror) {
-disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
-save = true;
-}
+} else {
+if (disk-mirror) {
+disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
+save = true;
+}
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), device, async);
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-ret = -1;
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), device,
modern);
+if (qemuDomainObjExitMonitor(driver, vm) 0) {
+ret = -1;
+goto endjob;
+}
-if (ret 0) {
-if (disk-mirror)
-disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
-goto endjob;
+if (ret 0) {
+if (disk-mirror)
+disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+goto endjob;
+}
}
- waitjob:
/* If we have made changes to XML due to a copy job, make a best
* effort to save it now. But we can ignore failure, since there
* will be further changes when the event marks completion. */
@@ -16372,33 +16367,38 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
* while still holding the VM job, to prevent newly scheduled
* block jobs from confusing us. */
if (!async) {
-/* Older qemu that lacked async reporting also lacked
- * blockcopy and active commit, so we can hardcode the
- * event to pull, and we know the XML doesn't need
- * updating. We have to generate two event variants. */
-int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
-int status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
-event = virDomainEventBlockJobNewFromObj(vm, disk-src-path, type,
- status);
-event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type,
- status);
-} else if (disk-blockJobSync) {
-/* XXX If the event reports failure, we should reflect
- * that back into the return status of this API call. */
-
-while (disk-blockJobStatus == -1 disk-blockJobSync) {
-
[libvirt] [PATCH 02/11] qemu: domain: Introduce helper to retrieve domain monitor object
In some cases where the function does not need to access the private
data this helper may be used to retrieve the monitor object.
---
src/qemu/qemu_domain.c | 13 +
src/qemu/qemu_domain.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 758fcd9..707ef8b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3025,3 +3025,16 @@ qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr
mem)
{
mem-size = VIR_ROUND_UP(mem-size, 1024);
}
+
+
+/**
+ * qemuDomainGetMonitor:
+ * @vm: domain object
+ *
+ * Returns the monitor pointer corresponding to the domain object @vm.
+ */
+qemuMonitorPtr
+qemuDomainGetMonitor(virDomainObjPtr vm)
+{
+return ((qemuDomainObjPrivatePtr) vm-privateData)-mon;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b854b54..33dac39 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -248,6 +248,8 @@ void qemuDomainObjDiscardAsyncJob(virQEMUDriverPtr driver,
virDomainObjPtr obj);
void qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj);
+qemuMonitorPtr qemuDomainGetMonitor(virDomainObjPtr vm)
+ATTRIBUTE_NONNULL(1);
void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
virDomainObjPtr obj)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] libxl: Move job acquisition in libxlDomainStart to callers
Martin Kletzander wrote:
On Wed, Mar 25, 2015 at 02:08:34PM -0600, Jim Fehlig wrote:
Let callers of libxlDomainStart decide when it is appropriate to
acquire a job on the associated virDomainObj.
This makes sense, I see many bugs this fixes, but how come
libxlDomainShutdownThread(), libxlDomainRestoreFlags() and
libxlDoMigrateReceive() don't need to start the job when they all call
libxlDomainStart()?
Commit 0521d658 starts a job for libxlDomainShutdownThread. This patch
adds starting a job in libxlDomainRestoreFlags. For migration, I'll
need to work on a follow-up series that fixes job handling in general.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
src/libxl/libxl_domain.c | 24 --
src/libxl/libxl_driver.c | 53
+++-
2 files changed, 52 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 05f6eb1..da4c1c7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -885,17 +893,26 @@ libxlDomainCreateXML(virConnectPtr conn, const
char *xml,
goto cleanup;
def = NULL;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+goto cleanup;
+}
+
This should be acquired before virDomainObjListAdd() since you need to
check whether it's active after creating the job.
Acquiring the job requires a virDomainObj, which we get from the call to
virDomainObjListAdd().
If I'm wrong, then
virDomainObjListRemove() should be only called if the vm is not
persistent (as CreateXML can be called on persistent ones as well),
shouldn't it?
Yes, I think you are right. This was coded up assuming CreateXML only
handled transient domains.
[...]
@@ -1695,11 +1712,20 @@ libxlDomainRestoreFlags(virConnectPtr conn,
const char *from,
def = NULL;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) {
+if (!vm-persistent) {
+virDomainObjListRemove(driver-domains, vm);
+vm = NULL;
+}
+goto cleanup;
+}
+
Same here, I guess.
Yes :-). A virDomainObj is needed to acquire a job.
Thanks for the review. I'll fix calling virDomainObjListRemove() on
persistent domains in V2.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] Clean up headers in src/util/virutil.h
On Wed, Apr 01, 2015 at 17:32:31 +0200, Ján Tomko wrote: * verify.h from gnulib is included in internal.h * sys/select.h is no longer needed added by commit da196338 to use fd_set in virExec prototype --- src/util/virutil.h | 2 -- 1 file changed, 2 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/6] storage: handle scsi/iscsi error paths better
v1 here: http://www.redhat.com/archives/libvir-list/2015-March/msg01562.html changes: 1/6: Removed the virGetLastError() == NULL check 2/6: New - reading more closely showed an error path without a goto 3/6: New - Adjust return for virStorageBackendSCSINewLun to be able to differentiate real error vs. deciding to return because a stable path could not be determined 4/6: New - Found an unused variable in processLU 5/6: Adjust the logic to return -1 for real errors and -2 for errors that are recoverable. Removed the need for goto cleanup. 6/6: Add check for non-existent stable path at startup (and fail). Add check during refresh for a found non standard stable path that returns no volumes in the pool John Ferlan (6): iscsi: Use error message from virStorageBackendSCSIFindLUs iscsi: Fix exit path for virStorageBackendISCSIFindLUs failure scsi: Adjust return value for virStorageBackendSCSINewLun scsi: Remove unused 'type_path' in processLU scsi: Adjust return values from processLU iscsi: Add checks for non standard stable target.path src/storage/storage_backend_iscsi.c | 33 - src/storage/storage_backend_scsi.c | 59 - 2 files changed, 64 insertions(+), 28 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/6] scsi: Adjust return values from processLU
Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.
After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a fatal message occurs rather than continuting processing
for (well) no apparent reason. It will also only set the *found value when
at least one of the processLU's was successful.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_scsi.c | 44 +++---
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index e085da2..8087960 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -375,6 +375,16 @@ getBlockDevice(uint32_t host,
}
+/*
+ * Process a Logical Unit entry from the scsi host device directory
+ *
+ * Returns:
+ *
+ * 0 = Found a valid entry
+ * -1 = Some sort of fatal error
+ * -2 = A non-fatal error such as a non disk/lun entry or an entry
+ *without a block device found
+ */
static int
processLU(virStoragePoolObjPtr pool,
uint32_t host,
@@ -393,7 +403,7 @@ processLU(virStoragePoolObjPtr pool,
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to determine if %u:%u:%u:%u is a
Direct-Access LUN),
host, bus, target, lun);
-goto out;
+return -1;
}
/* We don't create volumes for devices other than disk and cdrom
@@ -401,32 +411,25 @@ processLU(virStoragePoolObjPtr pool,
* isn't an error, either. */
if (!(device_type == VIR_STORAGE_DEVICE_TYPE_DISK ||
device_type == VIR_STORAGE_DEVICE_TYPE_ROM))
-{
-retval = 0;
-goto out;
-}
+return -2;
VIR_DEBUG(%u:%u:%u:%u is a Direct-Access LUN,
host, bus, target, lun);
if (getBlockDevice(host, bus, target, lun, block_device) 0) {
VIR_DEBUG(Failed to find block device for this LUN);
-goto out;
+return -2;
}
-if (virStorageBackendSCSINewLun(pool,
-host, bus, target, lun,
-block_device) 0) {
+retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun,
+ block_device);
+if (retval 0)
VIR_DEBUG(Failed to create new storage volume for %u:%u:%u:%u,
host, bus, target, lun);
-goto out;
-}
-retval = 0;
-
-VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
- host, bus, target, lun);
+else
+VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
+ host, bus, target, lun);
- out:
VIR_FREE(block_device);
return retval;
}
@@ -460,6 +463,8 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr
pool,
*found = false;
while ((retval = virDirRead(devicedir, lun_dirent, device_path)) 0) {
+int rc;
+
if (sscanf(lun_dirent-d_name, devicepattern,
bus, target, lun) != 3) {
continue;
@@ -467,7 +472,12 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr
pool,
VIR_DEBUG(Found possible LU '%s', lun_dirent-d_name);
-if (processLU(pool, scanhost, bus, target, lun) == 0)
+rc = processLU(pool, scanhost, bus, target, lun);
+if (rc == -1) {
+retval = -1;
+break;
+}
+if (rc == 0)
*found = true;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/6] iscsi: Add checks for non standard stable target.path
https://bugzilla.redhat.com/show_bug.cgi?id=1171933
If a non stable path is provided for the pool's target path, check to
see if the directory exists before allowing pool startup; otherwise,
later in the processLU calls to find LUN's all that happens is the
volume target.path will get the strdup'd value of the pool target.path
(which doesn't exist), so attempts to find the LU are unsuccessful
resulting in a started pool with no devices listed even though the
block devices for the iSCSI LU's do exist.
Additionally if the non stable path does exist and it's determined no
targets are found, then force failure in the refresh path.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_iscsi.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index fba037f..b5a15b1 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -149,6 +149,15 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
if (virStorageBackendSCSIFindLUs(pool, host) 0)
goto cleanup;
+if (pool-volumes.count == 0
+!STRPREFIX(pool-def-target.path, /dev)) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(Non stable target path '%s' for pool '%s'
+ found no target volumes),
+ pool-def-target.path, pool-def-name);
+return -1;
+}
+
retval = 0;
cleanup:
@@ -393,6 +402,15 @@ virStorageBackendISCSIStartPool(virConnectPtr conn,
return -1;
}
+if (!STRPREFIX(pool-def-target.path, /dev)
+!virFileExists(pool-def-target.path)) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(Non stable target path '%s' not found for pool
'%s'),
+ pool-def-target.path, pool-def-name);
+return -1;
+}
+
+
if ((session = virStorageBackendISCSISession(pool, true)) == NULL) {
if ((portal = virStorageBackendISCSIPortal(pool-def-source)) ==
NULL)
goto cleanup;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] hostdev: fix net config restore error
On 04/01/2015 12:15 PM, Huanle Han wrote:
On 2015年03月31日 17:46, Martin Kletzander wrote:
On Thu, Mar 26, 2015 at 10:23:47PM +0800, Huanle Han wrote:
Fix for such a case:
1. Domain A and B xml contain the same SRIOV net hostdev(interface
type='hostdev' / with same pci address).
2. virsh start A (Successfully, and configure the SRIOV net with
custom mac)
3. virsh start B (Fail because of the hostdev used by domain A or other
reason.)
In step 3, 'virHostdevNetConfigRestore' is called for the hostdev
which is still used by domain A. It makes the mac/vlan of the SRIOV net
change.
Makes perfect sense, thanks for finding that out.
Someone else encountered this last month too, and posted a patch:
https://www.redhat.com/archives/libvir-list/2015-February/msg00394.html
The patch was NACKed because I didn't think the fix was correct
(although it did work)
https://www.redhat.com/archives/libvir-list/2015-February/msg00976.html
I don't have the time to do a close analysis of this patch right now,
but from the comments it sounds like it addresses the concern that I had
with the previous patch (that it wouldn't be messing with any devices
that were currently not in use by any domain, and hadn't yet been
reached in the setup part). I wanted to point that out now so that
whoever looks at the next version of this patch checks for that.
Code Change in this fix:
1. As the pci used by other domain have been removed from
'pcidevs' in previous loop, we only restore the nic config for
the hostdev still in 'pcidevs'(used by this domain)
2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the
hostdev is a pci net device or not.
3. update the comments to make it more clear
Signed-off-by: Huanle Han hanxue...@gmail.com
mailto:hanxue...@gmail.com
---
src/util/virhostdev.c | 52
---
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 83f567d..b04bae2 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -350,6 +350,14 @@ virHostdevNetDevice(virDomainHostdevDefPtr
hostdev, char **linkdev,
return ret;
}
+static int
+virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev)
+{
+return hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS
+hostdev-source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
+hostdev-parent.type == VIR_DOMAIN_DEVICE_NET
+hostdev-parent.data.net http://parent.data.net;
+}
static int
virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf,
@@ -481,10 +489,7 @@
virHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev,
/* This is only needed for PCI devices that have been defined
* using interface type='hostdev'. For all others, it is a NOP.
*/
-if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
-hostdev-source.subsys.type !=
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI ||
-hostdev-parent.type != VIR_DOMAIN_DEVICE_NET ||
-!hostdev-parent.data.net http://parent.data.net)
+if (!virHostdevIsPCINetDevice(hostdev))
return 0;
isvf = virHostdevIsVirtualFunction(hostdev);
@@ -781,19 +786,20 @@
virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
goto cleanup;
}
-/* Again 4 loops; mark all devices as inactive before reset
+/* Here are 4 loops; mark all devices as inactive before reset
* them and reset all the devices before re-attach.
* Attach mac and port profile parameters to devices
*/
+
+/* Loop 1: delete the copy of the dev from pcidevs if it's used by
+ * other domain. Or delete it from activePCIHostDevs if it had
+ * been used by this domain.
+ */
i = 0;
while (i virPCIDeviceListCount(pcidevs)) {
virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
virPCIDevicePtr activeDev = NULL;
-/* delete the copy of the dev from pcidevs if it's used by
- * other domain. Or delete it from activePCIHostDevs if it had
- * been used by this domain.
- */
activeDev =
virPCIDeviceListFind(hostdev_mgr-activePCIHostdevs, dev);
if (activeDev) {
const char *usedby_drvname;
@@ -814,14 +820,27 @@
virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
* pcidevs, but has been removed from activePCIHostdevs.
*/
-/*
- * For SRIOV net host devices, unset mac and port profile before
- * reset and reattach device
+/* Loop 2: before reset and reattach device,
+ * unset mac and port profile for SRIOV net host devices used
by this domain
*/
The comments were formatted as a sentence, it would be nice to keep it
that way.
Is comment as below OK?
/*
* Loop 2: For SRIOV net host devices used by this domain, unset mac
* and port profile before reset and reattach device
*/
-for (i = 0; i
Re: [libvirt] [PATCH 3/6] Do not include cpu_map.h in libvirtd.c
On Wed, Apr 01, 2015 at 17:32:30 +0200, Ján Tomko wrote: No longer needed after commit dd47723 --- daemon/libvirtd.c | 1 - 1 file changed, 1 deletion(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3] libxl: fix dom0 balloon logic
Recent testing on large memory systems revealed a bug in the Xen xl
tool's freemem() function. When autoballooning is enabled, freemem()
is used to ensure enough memory is available to start a domain,
ballooning dom0 if necessary. When ballooning large amounts of memory
from dom0, freemem() would exceed its self-imposed wait time and
return an error. Meanwhile, dom0 continued to balloon. Starting the
domain later, after sufficient memory was ballooned from dom0, would
succeed. The libvirt implementation in libxlDomainFreeMem() suffers
the same bug since it is modeled after freemem().
In the end, the best place to fix the bug on the Xen side was to
slightly change the behavior of libxl_wait_for_memory_target().
Instead of failing after caller-provided wait_sec, the function now
blocks as long as dom0 memory ballooning is progressing. It will return
failure only when more memory is needed to reach the target and wait_sec
have expired with no progress being made. See xen.git commit fd3aa246.
There was a dicussion on how this would affect other libxl apps like
libvirt
http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html
If libvirt containing this patch was build against a Xen containing
the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem()
will fail after 30 sec and domain creation will be terminated.
Without this patch and with old libxl_wait_for_memory_target() behavior,
libxlDomainFreeMem() does not succeed after 30 sec, but returns success
anyway. Domain creation continues resulting in all sorts of fun stuff
like cpu soft lockups in the guest OS. It was decided to properly fix
libxl_wait_for_memory_target(), and if anything improve the default
behavior of apps using the freemem reference impl in xl.
xl was patched to accommodate the change in libxl_wait_for_memory_target()
with xen.git commit 883b30a0. This patch does the same in the libxl
driver. While at it, I changed the logic to essentially match
freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner
IMO and will make it easier to spot future, potentially interesting
divergences.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V3:
Remove unneeded local variable 'ret' in libxlDomainFreeMem.
src/libxl/libxl_domain.c | 49
1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ce9e4d8..37b2b58 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -811,38 +811,33 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config
*d_config)
{
uint32_t needed_mem;
uint32_t free_mem;
-size_t i;
-int ret = -1;
int tries = 3;
int wait_secs = 10;
-if ((ret = libxl_domain_need_memory(ctx, d_config-b_info,
-needed_mem)) = 0) {
-for (i = 0; i tries; ++i) {
-if ((ret = libxl_get_free_memory(ctx, free_mem)) 0)
-break;
+if (libxl_domain_need_memory(ctx, d_config-b_info, needed_mem) 0)
+goto error;
-if (free_mem = needed_mem) {
-ret = 0;
-break;
-}
+do {
+if (libxl_get_free_memory(ctx, free_mem) 0)
+goto error;
-if ((ret = libxl_set_memory_target(ctx, 0,
- free_mem - needed_mem,
- /* relative */ 1, 0)) 0)
-break;
+if (free_mem = needed_mem)
+return 0;
-ret = libxl_wait_for_free_memory(ctx, 0, needed_mem,
- wait_secs);
-if (ret == 0 || ret != ERROR_NOMEM)
-break;
+if (libxl_set_memory_target(ctx, 0, free_mem - needed_mem,
+/* relative */ 1, 0) 0)
+goto error;
-if ((ret = libxl_wait_for_memory_target(ctx, 0, 1)) 0)
-break;
-}
-}
+if (libxl_wait_for_memory_target(ctx, 0, wait_secs) 0)
+goto error;
-return ret;
+tries--;
+} while (tries 0);
+
+ error:
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+ _(Failed to balloon domain0 memory));
+return -1;
}
static void
@@ -958,12 +953,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
cfg-ctx, d_config) 0)
goto endjob;
-if (cfg-autoballoon libxlDomainFreeMem(cfg-ctx, d_config) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(libxenlight failed to get free memory for domain
'%s'),
- d_config.c_info.name);
+if (cfg-autoballoon libxlDomainFreeMem(cfg-ctx, d_config) 0)
goto endjob;
-}
if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
vm-def,
Re: [libvirt] [PATCH] qemu: lifecycle: make agent-mode shutdown and reboot timeout
On 01.04.2015 11:13, zhang bo wrote:
When we shutdown/reboot a guest using agent-mode, if the guest itself blocks
infinitely,
libvirt would block in qemuAgentShutdown() forever.
Thus, we set a timeout for shutdown/reboot, from our experience, 60 seconds
would be fine.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
---
include/libvirt/libvirt-qemu.h | 1 +
src/qemu/qemu_agent.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 0c5d650..2bb8ee8 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -49,6 +49,7 @@ typedef enum {
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
} virDomainQemuAgentCommandTimeoutValues;
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index a7b3279..548d580 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1300,7 +1300,7 @@ int qemuAgentShutdown(qemuAgentPtr mon,
else
mon-await_event = QEMU_AGENT_EVENT_SHUTDOWN;
ret = qemuAgentCommand(mon, cmd, reply, false,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN);
virJSONValueFree(cmd);
virJSONValueFree(reply);
Looks good to me. ACK after freeze.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] Remove unnecessary includes from virsh.h
On Wed, Apr 01, 2015 at 17:32:33 +0200, Ján Tomko wrote: Include them in the files that need them instead. --- tools/virsh-domain.c | 1 + tools/virsh-network.c | 1 + tools/virsh-nodedev.c | 1 + tools/virsh-snapshot.c | 1 + tools/virsh.h | 2 -- 5 files changed, 4 insertions(+), 2 deletions(-) ACK, 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 2/6] Use the DEFAULT_DRIVER_DIR macro
On Wed, Apr 01, 2015 at 17:32:29 +0200, Ján Tomko wrote: Unused since commit bc2f42a0. Move it under the WITHOUT_DRIVER_MODULES #ifdef and start using it again. --- src/driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK after release. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/11] qemu: domain: Add helper to check block job support
We need to check that qemu supports block jobs in multiple places. Add a
helper to do the check.
---
src/qemu/qemu_domain.c | 30 ++
src/qemu/qemu_domain.h | 2 ++
2 files changed, 32 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 707ef8b..3fb497f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3038,3 +3038,33 @@ qemuDomainGetMonitor(virDomainObjPtr vm)
{
return ((qemuDomainObjPrivatePtr) vm-privateData)-mon;
}
+
+
+/**
+ * qemuDomainSupportsBlockJobs:
+ * @vm: domain object
+ * @modern: pointer to bool that returns whether modern block jobs are
supported
+ *
+ * Returns -1 in case when qemu does not support block jobs at all. Otherwise
+ * returns 0 and optionally fills @modern to denote that modern (async) block
+ * jobs are supported.
+ */
+int
+qemuDomainSupportsBlockJobs(virDomainObjPtr vm,
+bool *modern)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+bool async = virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC);
+bool sync = virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC);
+
+if (!sync !async) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(block jobs not supported with this QEMU binary));
+return -1;
+}
+
+if (modern)
+*modern = async;
+
+return 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 33dac39..ec76e91 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -424,6 +424,8 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+int qemuDomainSupportsBlockJobs(virDomainObjPtr vm, bool *modern)
+ATTRIBUTE_NONNULL(1);
bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk);
void qemuDomObjEndAPI(virDomainObjPtr *vm);
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/11] qemu: blockjob: Separate qemuDomainBlockJobAbort from qemuDomainBlockJobImpl
Sacrifice a few lines of code in favor of the code being more readable.
---
src/qemu/qemu_driver.c | 213 +--
src/qemu/qemu_migration.c| 8 +-
src/qemu/qemu_monitor.c | 18
src/qemu/qemu_monitor.h | 6 +-
src/qemu/qemu_monitor_json.c | 37 ++--
src/qemu/qemu_monitor_json.h | 5 +
6 files changed, 184 insertions(+), 103 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 44ee04f..7b7775d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16173,16 +16173,12 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
char *device = NULL;
int ret = -1;
bool async = false;
-virObjectEventPtr event = NULL;
-virObjectEventPtr event2 = NULL;
int idx;
virDomainDiskDefPtr disk;
virStorageSourcePtr baseSource = NULL;
unsigned int baseIndex = 0;
char *basePath = NULL;
char *backingPath = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-bool save = false;
unsigned long long speed = bandwidth;
if (!virDomainObjIsActive(vm)) {
@@ -16234,40 +16230,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
if (mode == BLOCK_JOB_PULL qemuDomainDiskBlockJobIsActive(disk))
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,
- _(pivot of disk '%s' requires an active copy job),
- disk-dst);
-goto endjob;
-}
-if (disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE
-disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- _(another job on disk '%s' is still being ended),
- disk-dst);
-goto endjob;
-}
-
-if (disk-mirror (flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
-ret = qemuDomainBlockPivot(driver, vm, device, disk);
-if (ret 0 async)
-goto endjob;
-goto waitjob;
-}
-if (disk-mirror) {
-disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
-save = true;
-}
-}
-
if (base
(virStorageFileParseChainIndex(disk-dst, base, baseIndex) 0 ||
!(baseSource = virStorageFileChainLookup(disk-src, disk-src,
@@ -16319,13 +16281,107 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
if (qemuDomainObjExitMonitor(driver, vm) 0)
ret = -1;
if (ret 0) {
-if (mode == BLOCK_JOB_ABORT disk-mirror)
-disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
goto endjob;
} else if (mode == BLOCK_JOB_PULL) {
disk-blockjob = true;
}
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+VIR_FREE(basePath);
+VIR_FREE(backingPath);
+VIR_FREE(device);
+qemuDomObjEndAPI(vm);
+return ret;
+}
+
+static int
+qemuDomainBlockJobAbort(virDomainPtr dom,
+const char *path,
+unsigned int flags)
+{
+virQEMUDriverPtr driver = dom-conn-privateData;
+char *device = NULL;
+virObjectEventPtr event = NULL;
+virObjectEventPtr event2 = NULL;
+virDomainDiskDefPtr disk;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+bool save = false;
+int idx;
+bool async;
+virDomainObjPtr vm;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC |
+ VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, -1);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+if (virDomainBlockJobAbortEnsureACL(dom-conn, vm-def) 0)
+goto cleanup;
+
+if (qemuDomainSupportsBlockJobs(vm, async) 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain is not running));
+goto endjob;
+}
+
+if (!(device = qemuDiskPathToAlias(vm, path, idx)))
+goto endjob;
+disk = vm-def-disks[idx];
+
+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,
+ _(pivot of disk '%s' requires an active copy job),
+ disk-dst);
+goto endjob;
+}
+if
Re: [libvirt] [PATCH 1/6] Remove unused macros
On Wed, Apr 01, 2015 at 17:32:28 +0200, Ján Tomko wrote: In the order of appearance: * MAX_LISTEN - never used added by 23ad665c (qemud) and addec57 (lock daemon) * NEXT_FREE_CLASS_ID - never used, added by 07d1b6b * virLockError - never used, added by eb8268a4 * OPENVZ_MAX_ARG, CMDBUF_LEN, CMDOP_LEN unused since the removal of ADD_ARG_LIT in d8b31306 * QEMU_NB_PER_CPU_STAT_PARAM - unused since 897808e * QEMU_CMD_PROMPT, QEMU_PASSWD_PROMPT - unused since 1dc10a7 * TEST_MODEL_WORDSIZE - unused since c25c18f7 * TEMPDIR - never used, added by 714bef5 * NSIG - workaround around old headers added by commit 60ed1d2 unused since virExec was moved by commit 02e8691 * DO_TEST_PARSE - never used, added by 9afa006 * DIFF_MSEC, GETTIMEOFDAY - unused since eee6eb6 Wow, you bothered with electronic archeology? :) --- daemon/libvirtd.c | 1 - src/conf/network_conf.c | 1 - src/locking/lock_daemon.c | 1 - src/locking/lock_driver_lockd.c | 4 src/openvz/openvz_driver.c | 4 src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor_text.c| 3 --- src/test/test_driver.c | 1 - src/uml/uml_driver.c| 3 --- src/util/virutil.c | 4 tests/sockettest.c | 10 -- tests/testutils.c | 5 - 12 files changed, 39 deletions(-) ACK (after the release) Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/6] scsi: Remove unused 'type_path' in processLU
Seems to be a remnant that was never cleaned up from original submit...
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_scsi.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 6def373..e085da2 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -382,7 +382,6 @@ processLU(virStoragePoolObjPtr pool,
uint32_t target,
uint32_t lun)
{
-char *type_path = NULL;
int retval = -1;
int device_type;
char *block_device = NULL;
@@ -427,8 +426,6 @@ processLU(virStoragePoolObjPtr pool,
VIR_DEBUG(Created new storage volume for %u:%u:%u:%u successfully,
host, bus, target, lun);
-VIR_FREE(type_path);
-
out:
VIR_FREE(block_device);
return retval;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] Remove unused includes from virsh
On Wed, Apr 01, 2015 at 17:32:32 +0200, Ján Tomko wrote: After splitting out most of virsh command, some includes are no longer needed. Some files have the libXML includes despite not needing them. --- tools/virsh-network.c | 6 -- tools/virsh-nodedev.c | 6 -- tools/virsh-nwfilter.c | 6 -- tools/virsh-pool.c | 6 -- tools/virsh-secret.c | 6 -- tools/virsh.c | 10 -- tools/virsh.h | 1 - 7 files changed, 41 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
(CCing libvir-list and Jiri Denemark for libvirt-related discussion
about -cpu host/none, and live-migration safety expectations)
On Wed, Apr 01, 2015 at 06:31:23PM +0200, Michael Mueller wrote:
On Wed, 1 Apr 2015 10:01:13 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote:
On Tue, 31 Mar 2015 15:35:26 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
This patch implements a new QMP request named 'query-cpu-model'.
It returns the cpu model of cpu 0 and its backing accelerator.
request:
{execute : query-cpu-model }
answer:
{return : {name: 2827-ga2, accel: kvm }}
Alias names are resolved to their respective machine type and GA names
already during cpu instantiation. Thus, also a cpu model like 'host'
which is implemented as alias will return its normalized cpu model
name.
Furthermore the patch implements the following function:
- s390_cpu_models_used(), returns true if S390 cpu models are in use
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
---
[...]
+static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
+{
+return g_strdup_printf(%04x-ga%u, cc-proc.type, cc-mach.ga);
+}
How exactly is this information going to be used by clients? If getting
the correct type and ga values is important for them, maybe you could
add them as integer fields, instead of requiring clients to parse the
CPU model name?
The consumer don't need to parse the name, it is just important for them
to have
distinctive names that correlate with the names returned by
query-cpu-definitions.
Once the name of an active guest is known, e.g. (2827-ga2, kvm) a
potential
migration target can be verified, i.e. its query-cpu-definitions answer
for kvm
has to contain 2827-ga2 with the attribute runnable set to true. With
that mechanism
also the largest common denominator can be calculated. That model will be
used then.
Understood. So the point is to really have a name that can be found at
query-cpu-definitions. Makes sense.
(BTW, if you reused strdup_s390_cpu_name() inside
s390_cpu_compare_class_name() too, you would automatically ensure that
query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will
always agree with each other).
I have to verify but it seems to make sense from reading... I will do that if
it works. :-)
I also changed the above mentioned routine to map the cpu model none case:
static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
{
if (cpuid(cc-proc)) {
return g_strdup_printf(%04x-ga%u, cc-proc.type, cc-mach.ga);
} else {
return g_strdup(none);
}
}
What about:
static const char *s390_cpu_name(S390CPUClass *cc)
{
return cc-model_name;
}
And then you can just set cc-model_name=_name inside S390_PROC_DEF (and
set it to none inside s390_cpu_class_init()).
Wouldn't that store redundant information... but it would at least shift the
work into the
initialization phase and do the format just once per model.
To be honest, calculating the CPU model name on the fly with
strdup_s390_cpu_name() like you did above wouldn't be a problem to me.
I just wanted to avoid having the same CPU model name logic (and special
cases like none) duplicated inside strdup_s390_cpu_name(),
S390_PROC_DEF, s390_cpu_class_by_name(), and maybe other places. Maybe
this duplication can be eliminated by simply reusing
strdup_s390_cpu_name() inside s390_cpu_compare_class_name().
I wonder if this class-model_name conversion could be made generic
inside the CPU class. We already have a CPU::class_by_name() method, so
it makes sense to have the opposite function too.
(But I wouldn't mind making this s390-specific first, and converted
later to generic code if appropriate).
ok
This implicitly will fail a comparison for cpu model (none, kvm) as
that will
never be part of the query-cpu-definitions answer.
I am not sure I follow. If (none, kvm) is never in the list, is
-cpu none -machine accel=kvm always an invalid use case?
Not directly invalid as -cpu none will be the same as omitting the -cpu
option.
KVM will setup the vcpu properties withou any QEMU control to whatever the
hosting
machine and the kvm kernel code offers. That will allow to run QEMU against a
KVM
version that is not aware of the s390 cpu model ioctls.
It looks like we have conflicting expectations about
query-cpu-definitions, it seems: on the one hand, if -cpu none is
valid I believe it should appear on the query-cpu-definitions return
value; on the other hand, it is not (always?) migration-safe, so just
comparing the source query-cpus data with
[libvirt] [PATCH 04/11] qemu: blockjob: Use the new helpers in qemuDomainGetBlockJobInfo
Refactor the function to use the new helpers.
---
src/qemu/qemu_driver.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index becf415..6a2b58d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16457,7 +16457,6 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const char
*path,
virDomainBlockJobInfoPtr info, unsigned int flags)
{
virQEMUDriverPtr driver = dom-conn-privateData;
-qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
char *device = NULL;
int idx;
@@ -16470,18 +16469,11 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const
char *path,
if (!(vm = qemuDomObjFromDomain(dom)))
return -1;
-if (virDomainGetBlockJobInfoEnsureACL(dom-conn, vm-def) 0) {
-qemuDomObjEndAPI(vm);
-return -1;
-}
+if (virDomainGetBlockJobInfoEnsureACL(dom-conn, vm-def) 0)
+goto cleanup;
-priv = vm-privateData;
-if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)
-!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(block jobs not supported with this QEMU binary));
+if (qemuDomainSupportsBlockJobs(vm, NULL) 0)
goto cleanup;
-}
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) 0)
goto cleanup;
@@ -16492,13 +16484,13 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const
char *path,
goto endjob;
}
-device = qemuDiskPathToAlias(vm, path, idx);
-if (!device)
+if (!(device = qemuDiskPathToAlias(vm, path, idx)))
goto endjob;
disk = vm-def-disks[idx];
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJobInfo(priv-mon, device, info, bandwidth);
+ret = qemuMonitorBlockJobInfo(qemuDomainGetMonitor(vm), device, info,
+ bandwidth);
if (qemuDomainObjExitMonitor(driver, vm) 0)
ret = -1;
if (ret 0)
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/11] qemu: blockjob: Split qemuDomainBlockJobSetSpeed from qemuDomainBlockJobImpl
qemuDomainBlockJobImpl become an unmaintainable mess over the years of
adding new stuff to it. This patch starts splitting up individual
functions from it until it can be killed entirely.
In bulk this will add lines of code rather than delete them but it will
be traded for maintainability.
---
src/qemu/qemu_driver.c | 66
src/qemu/qemu_monitor.c | 19 +
src/qemu/qemu_monitor.h | 6 +++-
src/qemu/qemu_monitor_json.c | 41 +--
src/qemu/qemu_monitor_json.h | 6
5 files changed, 118 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6a2b58d..eec5457 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16339,10 +16339,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
/* Convert bandwidth MiB to bytes, if needed */
-if ((mode == BLOCK_JOB_SPEED
- !(flags VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES)) ||
-(mode == BLOCK_JOB_PULL
- !(flags VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES))) {
+if (mode == BLOCK_JOB_PULL
+!(flags VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES)) {
if (speed LLONG_MAX 20) {
virReportError(VIR_ERR_OVERFLOW,
_(bandwidth must be less than %llu),
@@ -16532,23 +16530,69 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const
char *path,
return ret;
}
+
static int
-qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
- unsigned long bandwidth, unsigned int flags)
+qemuDomainBlockJobSetSpeed(virDomainPtr dom,
+ const char *path,
+ unsigned long bandwidth,
+ unsigned int flags)
{
+virQEMUDriverPtr driver = dom-conn-privateData;
+int ret = -1;
virDomainObjPtr vm;
+bool modern;
+const char *device;
+unsigned long long speed = bandwidth;
+
virCheckFlags(VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, -1);
+/* Convert bandwidth MiB to bytes, if needed */
+if (!(flags VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES)) {
+if (speed LLONG_MAX 20) {
+virReportError(VIR_ERR_OVERFLOW,
+ _(bandwidth must be less than %llu),
+ LLONG_MAX 20);
+return -1;
+}
+speed = 20;
+}
+
if (!(vm = qemuDomObjFromDomain(dom)))
return -1;
-if (virDomainBlockJobSetSpeedEnsureACL(dom-conn, vm-def) 0) {
-qemuDomObjEndAPI(vm);
-return -1;
+if (virDomainBlockJobSetSpeedEnsureACL(dom-conn, vm-def) 0)
+goto cleanup;
+
+if (qemuDomainSupportsBlockJobs(vm, modern) 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain is not running));
+goto endjob;
}
-return qemuDomainBlockJobImpl(vm, dom-conn, path, NULL, bandwidth,
- BLOCK_JOB_SPEED, flags);
+if (!(device = qemuDiskPathToAlias(vm, path, NULL)))
+goto endjob;
+
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorBlockJobSetSpeed(qemuDomainGetMonitor(vm),
+ device,
+ speed,
+ modern);
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+ret = -1;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+qemuDomObjEndAPI(vm);
+
+return ret;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5da0b8f..aae259f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3606,6 +3606,25 @@ qemuMonitorBlockJob(qemuMonitorPtr mon,
int
+qemuMonitorBlockJobSetSpeed(qemuMonitorPtr mon,
+const char *device,
+unsigned long long bandwidth,
+bool modern)
+{
+VIR_DEBUG(mon=%p, device=%s, bandwidth=%lluB, modern=%d,
+ mon, device, bandwidth, modern);
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(block jobs require JSON monitor));
+return -1;
+}
+
+return qemuMonitorJSONBlockJobSetSpeed(mon, device, bandwidth, modern);
+}
+
+
+int
qemuMonitorBlockJobInfo(qemuMonitorPtr mon,
const char *device,
virDomainBlockJobInfoPtr info,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index c7c3dab..9cd0d9d 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -759,7 +759,6 @@ int qemuMonitorSendKey(qemuMonitorPtr mon,
typedef enum {
BLOCK_JOB_ABORT,
-BLOCK_JOB_SPEED,
BLOCK_JOB_PULL,
} qemuMonitorBlockJobCmd;
@@ -772,6 +771,11 @@ int
[libvirt] [PATCH 00/11] qemu: Refactor the block job code
At the expense of adding 113 lines of code, kill the ugly qemuBlockJobImpl method and spread it's guts into separate functions. This series additionally fixes a issue with failed drive pivot and the abort function now returns errors if the returned event contained failure. Peter Krempa (11): qemu: monitor: Extract handling of JSON block job error codes qemu: domain: Introduce helper to retrieve domain monitor object qemu: domain: Add helper to check block job support qemu: blockjob: Use the new helpers in qemuDomainGetBlockJobInfo qemu: blockjob: Split qemuDomainBlockJobSetSpeed from qemuDomainBlockJobImpl qemu: Clean up old leftovers in qemuMonitorDrivePivot qemu: blockPivot: Don't pause the VM any more since we don't use drive-reopen qemu: blockjob: Separate qemuDomainBlockJobAbort from qemuDomainBlockJobImpl qemu: blockPull: Refactor the rest of qemuDomainBlockJobImpl qemu: drivePivot: Fix assumption when 'block-job-complete' fails qemu: Refactor qemuDomainBlockJobAbort() src/qemu/qemu_domain.c | 43 + src/qemu/qemu_domain.h | 4 + src/qemu/qemu_driver.c | 437 +-- src/qemu/qemu_migration.c| 8 +- src/qemu/qemu_monitor.c | 88 ++--- src/qemu/qemu_monitor.h | 35 ++-- src/qemu/qemu_monitor_json.c | 188 +++ src/qemu/qemu_monitor_json.h | 28 ++- tests/qemumonitorjsontest.c | 2 +- 9 files changed, 473 insertions(+), 360 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/11] qemu: Clean up old leftovers in qemuMonitorDrivePivot
There are two leftover unused variables. Remove them and clean up the
fallout of the change.
---
src/qemu/qemu_driver.c | 5 +
src/qemu/qemu_monitor.c | 21 +
src/qemu/qemu_monitor.h | 6 ++
src/qemu/qemu_monitor_json.c | 5 ++---
src/qemu/qemu_monitor_json.h | 4 +---
tests/qemumonitorjsontest.c | 2 +-
6 files changed, 16 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eec5457..7c33ca3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16050,7 +16050,6 @@ qemuDomainBlockPivot(virConnectPtr conn,
int ret = -1, rc;
qemuDomainObjPrivatePtr priv = vm-privateData;
virDomainBlockJobInfo info;
-const char *format = NULL;
bool resume = false;
virStorageSourcePtr oldsrc = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -16062,8 +16061,6 @@ qemuDomainBlockPivot(virConnectPtr conn,
goto cleanup;
}
-format = virStorageFileFormatTypeToString(disk-mirror-format);
-
/* Probe the status, if needed. */
if (!disk-mirrorState) {
qemuDomainObjEnterMonitor(driver, vm);
@@ -16147,7 +16144,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
* overall return value. */
disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT;
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorDrivePivot(priv-mon, device, disk-mirror-path, format);
+ret = qemuMonitorDrivePivot(priv-mon, device);
if (qemuDomainObjExitMonitor(driver, vm) 0) {
ret = -1;
goto cleanup;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index aae259f..0e50dbb 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3492,23 +3492,20 @@ qemuMonitorDiskNameLookup(qemuMonitorPtr mon,
}
-/* Use the block-job-complete monitor command to pivot a block copy
- * job. */
+/* Use the block-job-complete monitor command to pivot a block copy job. */
int
-qemuMonitorDrivePivot(qemuMonitorPtr mon, const char *device,
- const char *file, const char *format)
+qemuMonitorDrivePivot(qemuMonitorPtr mon,
+ const char *device)
{
-int ret = -1;
-
-VIR_DEBUG(mon=%p, device=%s, file=%s, format=%s,
- mon, device, file, NULLSTR(format));
+VIR_DEBUG(mon=%p, device=%s, mon, device);
-if (mon-json)
-ret = qemuMonitorJSONDrivePivot(mon, device, file, format);
-else
+if (mon-json) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(drive pivot requires JSON monitor));
-return ret;
+return -1;
+}
+
+return qemuMonitorJSONDrivePivot(mon, device);
}
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 9cd0d9d..78f4648 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -721,10 +721,8 @@ int qemuMonitorDriveMirror(qemuMonitorPtr mon,
unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int qemuMonitorDrivePivot(qemuMonitorPtr mon,
- const char *device,
- const char *file,
- const char *format)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+ const char *device)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *device,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 34a6d56..e0f16ec 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3859,9 +3859,8 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char
*device,
}
int
-qemuMonitorJSONDrivePivot(qemuMonitorPtr mon, const char *device,
- const char *file ATTRIBUTE_UNUSED,
- const char *format ATTRIBUTE_UNUSED)
+qemuMonitorJSONDrivePivot(qemuMonitorPtr mon,
+ const char *device)
{
int ret;
virJSONValuePtr cmd;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 09f898c..5185bbf 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -264,9 +264,7 @@ int qemuMonitorJSONDriveMirror(qemuMonitorPtr mon,
unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon,
- const char *device,
- const char *file,
- const char *format)
+ const char *device)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index f09176a..170d690 100644
---
[libvirt] [PATCH 10/11] qemu: drivePivot: Fix assumption when 'block-job-complete' fails
QEMU does not abandon the mirror. The job carries on in the synchronised
phase and it might be either pivoted again or cancelled. The commit
hints that the described behavior was happening in a downstream version.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1202704
---
Notes:
I've spent some time looking how the active commit and copy job actually
works in qemu, but I did not check if that behavior changed in the upstream
releases. At any rate, it makes sense thus I expect that it was there
ever-since.
src/qemu/qemu_driver.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2dd8ed4..e9431ad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16128,27 +16128,10 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
}
if (ret 0) {
-/* On failure, qemu abandons the mirror, and reverts back to
- * the source disk (RHEL 6.3 has a bug where the revert could
- * cause catastrophic failure in qemu, but we don't need to
- * worry about it here as it is not an upstream qemu problem. */
-/* XXX should we be parsing the exact qemu error, or calling
- * 'query-block', to see what state we really got left in
- * before killing the mirroring job?
- * XXX We want to revoke security labels and disk lease, as
- * well as audit that revocation, before dropping the original
- * source. But it gets tricky if both source and mirror share
- * common backing files (we want to only revoke the non-shared
- * portion of the chain); so for now, we leak the access to
- * the original. */
-virStorageSourceFree(disk-mirror);
-disk-mirror = NULL;
-disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
-disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
-disk-blockjob = false;
+/* The pivot failed. The block job in QEMU remains in the synchronised
+ * phase. Reset the state we changed and return the error to the user
*/
+disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
}
-if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0)
-ret = -1;
cleanup:
if (oldsrc)
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/11] qemu: monitor: Extract handling of JSON block job error codes
My intention is to split qemuMonitorJSONBlockJob() into simpler separate
functions for every block job type. Since the error handling code is the
same for all block jobs, this patch extracts the code into a separate
function that will later be reused in more places.
---
src/qemu/qemu_monitor_json.c | 64 +++-
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index aa844ca..edd0a3f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4252,6 +4252,39 @@ qemuMonitorJSONBlockJobInfo(qemuMonitorPtr mon,
}
+static int
+qemuMonitorJSONBlockJobError(virJSONValuePtr reply,
+ const char *cmd_name,
+ const char *device)
+{
+if (!virJSONValueObjectHasKey(reply, error))
+return 0;
+
+if (qemuMonitorJSONHasError(reply, DeviceNotActive)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(No active operation on device: %s), device);
+} else if (qemuMonitorJSONHasError(reply, DeviceInUse)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(Device %s in use), device);
+} else if (qemuMonitorJSONHasError(reply, NotSupported)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(Operation is not supported for device: %s), device);
+} else if (qemuMonitorJSONHasError(reply, CommandNotFound)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(Command '%s' is not found), cmd_name);
+} else {
+virJSONValuePtr error = virJSONValueObjectGet(reply, error);
+
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unexpected error: (%s) '%s'),
+ NULLSTR(virJSONValueObjectGetString(error, class)),
+ NULLSTR(virJSONValueObjectGetString(error, desc)));
+}
+
+return -1;
+}
+
+
/* speed is in bytes/sec */
int
qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
@@ -4322,34 +4355,15 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
if (!cmd)
return -1;
-ret = qemuMonitorJSONCommand(mon, cmd, reply);
+if (qemuMonitorJSONCommand(mon, cmd, reply) 0)
+goto cleanup;
-if (ret == 0 virJSONValueObjectHasKey(reply, error)) {
-ret = -1;
-if (qemuMonitorJSONHasError(reply, DeviceNotActive)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- _(No active operation on device: %s),
- device);
-} else if (qemuMonitorJSONHasError(reply, DeviceInUse)) {
-virReportError(VIR_ERR_OPERATION_FAILED,
- _(Device %s in use), device);
-} else if (qemuMonitorJSONHasError(reply, NotSupported)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- _(Operation is not supported for device: %s),
- device);
-} else if (qemuMonitorJSONHasError(reply, CommandNotFound)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- _(Command '%s' is not found), cmd_name);
-} else {
-virJSONValuePtr error = virJSONValueObjectGet(reply, error);
+if (qemuMonitorJSONBlockJobError(reply, cmd_name, device) 0)
+goto cleanup;
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Unexpected error: (%s) '%s'),
- NULLSTR(virJSONValueObjectGetString(error,
class)),
- NULLSTR(virJSONValueObjectGetString(error,
desc)));
-}
-}
+ret = 0;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/6] iscsi: Fix exit path for virStorageBackendISCSIFindLUs failure
If the call to virStorageBackendISCSIGetHostNumber failed, we set
retval = -1, but yet still called virStorageBackendSCSIFindLUs.
Need to add a goto cleanup - while at it, adjust the logic to
initialize retval to -1 and only changed to 0 (zero) on success.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_iscsi.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 48f0957..fba037f 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -131,23 +131,27 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
const char *session)
{
char *sysfs_path;
-int retval = 0;
+int retval = -1;
uint32_t host;
if (virAsprintf(sysfs_path,
/sys/class/iscsi_session/session%s/device, session) 0)
-return -1;
+goto cleanup;
if (virStorageBackendISCSIGetHostNumber(sysfs_path, host) 0) {
virReportSystemError(errno,
_(Failed to get host number for iSCSI session
with path '%s'),
sysfs_path);
-retval = -1;
+goto cleanup;
}
if (virStorageBackendSCSIFindLUs(pool, host) 0)
-retval = -1;
+goto cleanup;
+
+retval = 0;
+
+ cleanup:
VIR_FREE(sysfs_path);
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/6] scsi: Adjust return value for virStorageBackendSCSINewLun
Add a return -2 to differentiate that the failure was a result of a non stable device path found or some other real error which would be messaged in some manner. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 58e7e6d..6def373 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev) } +/* + * Attempt to create a new LUN + * + * Returns: + * + * 0 = Success + * -1 = Failure due to some sort of OOM or other fatal issue found when + *attempting to get/update information about a found volume + * -2 = Failure to find a stable path + */ static int virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, uint32_t host ATTRIBUTE_UNUSED, @@ -202,7 +212,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, VIR_DEBUG(No stable path found for '%s' in '%s', devpath, pool-def-target.path); -retval = -1; +retval = -2; goto free_vol; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/6] iscsi: Use error message from virStorageBackendSCSIFindLUs
Don't supercede the error message virStorageBackendSCSIFindLUs as the
message such as error: Failed to find LUs on host 60: ... is not overly
clear as to what the real problem might be.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/storage/storage_backend_iscsi.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 079c767..48f0957 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -146,11 +146,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
retval = -1;
}
-if (virStorageBackendSCSIFindLUs(pool, host) 0) {
-virReportSystemError(errno,
- _(Failed to find LUs on host %u), host);
+if (virStorageBackendSCSIFindLUs(pool, host) 0)
retval = -1;
-}
VIR_FREE(sysfs_path);
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On Wed, 1 Apr 2015 21:05:31 +0200
Michael Mueller m...@linux.vnet.ibm.com wrote:
And cpu model none just means that QEMU does not manage the cpu model.
That's also
the reason why I initially returned an empty [] model and not none. This
somewhat
convinces me to go back to this approach...
And for query-cpus that can be represented as a non-existent model field:
[{current:true,CPU:0,halted:false,thread_id:39110}, ...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On Wed, 1 Apr 2015 13:59:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: Not directly invalid as -cpu none will be the same as omitting the -cpu option. KVM will setup the vcpu properties withou any QEMU control to whatever the hosting machine and the kvm kernel code offers. That will allow to run QEMU against a KVM version that is not aware of the s390 cpu model ioctls. It looks like we have conflicting expectations about query-cpu-definitions, it seems: on the one hand, if -cpu none is valid I believe it should appear on the query-cpu-definitions return value; on the other hand, it is not (always?) migration-safe, so just comparing the source query-cpus data with the target query-cpu-definitions data wouldn't be enough to ensure live-migration safety. There are other cases as well where the value given with -cpu is *not* part of the cpu definition list and that is aliases: [mimu@p57lp59 (master-cpu-model) qemu]$ ./s390x-softmmu/qemu-system-s390x -machine s390-ccw,accel=kvm -cpu ? s390 2064-ga1 IBM zSeries 900 GA1 s390 2064-ga2 IBM zSeries 900 GA2 s390 2064-ga3 IBM zSeries 900 GA3 s390 2064 (alias for 2064-ga3) s390 z900 (alias for 2064-ga3) ... s390 z10(alias for 2097-ga3) s390 z10-ec (alias for 2097-ga3) s390 2098-ga1 IBM System z10 BC GA1 s390 2098-ga2 IBM System z10 BC GA2 s390 2098 (alias for 2098-ga2) s390 z10-bc (alias for 2098-ga2) s390 2817-ga1 IBM zEnterprise 196 GA1 s390 2817-ga2 IBM zEnterprise 196 GA2 s390 2817 (alias for 2817-ga2) s390 z196 (alias for 2817-ga2) s390 2818-ga1 IBM zEnterprise 114 GA1 s390 2818 (alias for 2818-ga1) s390 z114 (alias for 2818-ga1) s390 2827-ga1 IBM zEnterprise EC12 GA1 s390 2827-ga2 IBM zEnterprise EC12 GA2 s390 2827 (alias for 2827-ga2) s390 zEC12 (alias for 2827-ga2) s390 host (alias for 2827-ga2) s390 2828-ga1 IBM zEnterprise BC12 GA1 s390 2828 (alias for 2828-ga1) s390 zBC12 (alias for 2828-ga1) As you can see host is in s390x case always an alias and also all other aliases are normalized to their real cpu models in the cpu-definitions list. On x86, we have a similar problem with -cpu host, that changes depending on the host hardware and host kernel. We solve this problem by making libvirt code aware of the set of valid CPU models, and libvirt has special cases for -cpu host. -cpu host is not a special case for s390, it will return (2827-ga2, kvm) as cpu model or whatever model the hosting system implements. If you don't want to encode that knowledge in libvirt or other management software for s390, it looks like you need something like a stable-abi-safe field on CpuDefinitionInfo? Exactly that fulfills the name field for s390 already in my view. And cpu model none just means that QEMU does not manage the cpu model. That's also the reason why I initially returned an empty [] model and not none. This somewhat convinces me to go back to this approach... Michael -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH 2/3] libxl: acquire a job when destroying a domain
Martin Kletzander wrote: On Thu, Mar 26, 2015 at 03:29:51PM -0600, Jim Fehlig wrote: Konrad Rzeszutek Wilk wrote: On Wed, Mar 25, 2015 at 02:08:35PM -0600, Jim Fehlig wrote: A job should be acquired at the beginning of a domain destroy operation, not at the end when cleaning up the domain. Fix two occurances of this And s/occurances/occurrences/ here. It looks fine, though, with the squash-in. Thanks. I've sent a V2 addressing comments from you and Konrad https://www.redhat.com/archives/libvir-list/2015-April/msg00072.html Also, if you want to have a look at some other things that might be fixed here, plus some speed-up gained, have a look at my commit 540c339a, that does some similar things in the QEMU driver. Ah, that is nice work! I recall seeing the series and thinking it was pertinent to the libxl driver, but then forgot to take a closer look. Thanks for the reminder! I'll work on a similar improvement in the libxl driver. But IMO that should not hold up this series, which is a big improvement in itself. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/11 v2] qemu: drivePivot: Fix assumption when 'block-job-complete' fails
QEMU does not abandon the mirror. The job carries on in the synchronised
phase and it might be either pivoted again or cancelled. The commit
hints that the described behavior was happening in a downstream version.
If the command returns false there are two possible options:
1) qemu did not reach the point where it would ask the block job to
pivot
2) pivotting failed in the actual qemu coroutine
If either of those would happen we return failure and reset the
condition that waits for the block job to complete. This makes the API
fail but in case where qemu would actually abandon the mirror the fact
is notified via the event and handled asynchronously.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1202704
---
Notes:
I've spent some time looking how the active commit and copy job actually
works in qemu, but I did not check if that behavior changed in the upstream
releases. At any rate, it makes sense thus I expect that it was there
ever-since.
Version 2:
- this version resets the flag that makes libvirt wait on the event. This
should
make the API as rugged as it can possibly be.
src/qemu/qemu_driver.c | 27 ++-
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2dd8ed4..52c3587 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16128,27 +16128,10 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
}
if (ret 0) {
-/* On failure, qemu abandons the mirror, and reverts back to
- * the source disk (RHEL 6.3 has a bug where the revert could
- * cause catastrophic failure in qemu, but we don't need to
- * worry about it here as it is not an upstream qemu problem. */
-/* XXX should we be parsing the exact qemu error, or calling
- * 'query-block', to see what state we really got left in
- * before killing the mirroring job?
- * XXX We want to revoke security labels and disk lease, as
- * well as audit that revocation, before dropping the original
- * source. But it gets tricky if both source and mirror share
- * common backing files (we want to only revoke the non-shared
- * portion of the chain); so for now, we leak the access to
- * the original. */
-virStorageSourceFree(disk-mirror);
-disk-mirror = NULL;
-disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
-disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
-disk-blockjob = false;
+/* The pivot failed. The block job in QEMU remains in the synchronised
+ * phase. Reset the state we changed and return the error to the user
*/
+disk-mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
}
-if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0)
-ret = -1;
cleanup:
if (oldsrc)
@@ -16354,8 +16337,10 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
if (disk-mirror (flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
ret = qemuDomainBlockPivot(driver, vm, device, disk);
-if (ret 0 async)
+if (ret 0 async) {
+disk-blockJobSync = false;
goto endjob;
+}
goto waitjob;
}
if (disk-mirror) {
--
2.2.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 1/3] libxl: Move job acquisition in libxlDomainStart to callers
Let callers of libxlDomainStart decide when it is appropriate to
acquire a job on the associated virDomainObj.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V2:
Don't call virDomainObjListRemove() on persistent domain
src/libxl/libxl_domain.c | 24 -
src/libxl/libxl_driver.c | 55 +++-
2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ce9e4d8..6b9959f 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -908,16 +908,13 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
libxl_domain_config_init(d_config);
-if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0)
-return ret;
-
cfg = libxlDriverConfigGet(driver);
/* If there is a managed saved state restore it instead of starting
* from scratch. The old state is removed once the restoring succeeded. */
if (restore_fd 0) {
managed_save_path = libxlDomainManagedSavePath(driver, vm);
if (managed_save_path == NULL)
-goto endjob;
+goto cleanup;
if (virFileExists(managed_save_path)) {
@@ -925,7 +922,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
managed_save_path,
def, hdr);
if (managed_save_fd 0)
-goto endjob;
+goto cleanup;
restore_fd = managed_save_fd;
@@ -939,7 +936,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
_(cannot restore domain '%s' uuid %s from a
file
which belongs to domain '%s' uuid %s),
vm-def-name, vm_uuidstr, def-name,
def_uuidstr);
-goto endjob;
+goto cleanup;
}
virDomainObjAssignDef(vm, def, true, NULL);
@@ -956,18 +953,18 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver-reservedVNCPorts, vm-def,
cfg-ctx, d_config) 0)
-goto endjob;
+goto cleanup;
if (cfg-autoballoon libxlDomainFreeMem(cfg-ctx, d_config) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(libxenlight failed to get free memory for domain
'%s'),
d_config.c_info.name);
-goto endjob;
+goto cleanup;
}
if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
vm-def, VIR_HOSTDEV_SP_PCI) 0)
-goto endjob;
+goto cleanup;
/* Unlock virDomainObj while creating the domain */
virObjectUnlock(vm);
@@ -999,7 +996,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
virReportError(VIR_ERR_INTERNAL_ERROR,
_(libxenlight failed to restore domain '%s'),
d_config.c_info.name);
-goto endjob;
+goto cleanup;
}
/*
@@ -1046,7 +1043,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
libxlDomainEventQueue(driver, event);
ret = 0;
-goto endjob;
+goto cleanup;
cleanup_dom:
if (priv-deathW) {
@@ -1057,10 +1054,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
vm-def-id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
- endjob:
-if (!libxlDomainObjEndJob(driver, vm))
-vm = NULL;
-
+ cleanup:
libxl_domain_config_dispose(d_config);
VIR_FREE(dom_xml);
VIR_FREE(managed_save_path);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 05f6eb1..e315b32 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -303,18 +303,26 @@ libxlAutostartDomain(virDomainObjPtr vm,
virObjectLock(vm);
virResetLastError();
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) {
+virObjectUnlock(vm);
+return ret;
+}
+
if (vm-autostart !virDomainObjIsActive(vm)
libxlDomainStart(driver, vm, false, -1) 0) {
err = virGetLastError();
VIR_ERROR(_(Failed to autostart VM '%s': %s),
vm-def-name,
err ? err-message : _(unknown error));
-goto cleanup;
+goto endjob;
}
ret = 0;
- cleanup:
-virObjectUnlock(vm);
+
+ endjob:
+if (libxlDomainObjEndJob(driver, vm))
+virObjectUnlock(vm);
+
return ret;
}
@@ -885,17 +893,28 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
goto cleanup;
def = NULL;
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) {
+if (!vm-persistent) {
+
[libvirt] [PATCH V2 3/3] libxl: drop virDomainObj lock when destroying a domain
A destroy operation can take considerable time on large memory
domains due to scrubbing the domain' memory. The operation is
running in the context of a job, so unlocking the domain and
allowing query operations is safe.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V2:
Unchanged
src/libxl/libxl_domain.c | 4
src/libxl/libxl_driver.c | 5 -
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index faf2c19..0ac5c62 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -435,7 +435,9 @@ libxlDomainShutdownThread(void *opaque)
libxlDomainEventQueue(driver, dom_event);
dom_event = NULL;
}
+virObjectUnlock(vm);
libxl_domain_destroy(cfg-ctx, vm-def-id, NULL);
+virObjectLock(vm);
libxlDomainCleanup(driver, vm, reason);
if (!vm-persistent)
virDomainObjListRemove(driver-domains, vm);
@@ -447,7 +449,9 @@ libxlDomainShutdownThread(void *opaque)
libxlDomainEventQueue(driver, dom_event);
dom_event = NULL;
}
+virObjectUnlock(vm);
libxl_domain_destroy(cfg-ctx, vm-def-id, NULL);
+virObjectLock(vm);
libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
if (libxlDomainStart(driver, vm, false, -1) 0) {
virErrorPtr err = virGetLastError();
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c9623ef..9e08de3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1252,7 +1252,10 @@ libxlDomainDestroyFlags(virDomainPtr dom,
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
-if (libxl_domain_destroy(cfg-ctx, vm-def-id, NULL) 0) {
+virObjectUnlock(vm);
+ret = libxl_domain_destroy(cfg-ctx, vm-def-id, NULL);
+virObjectLock(vm);
+if (ret 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to destroy domain '%d'), vm-def-id);
goto endjob;
--
1.8.4.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 2/3] libxl: acquire a job when destroying a domain
A job should be acquired at the beginning of a domain destroy operation,
not at the end when cleaning up the domain. Fix two occurrences of this
late job acquisition in the libxl driver. Doing so renders
libxlDomainCleanupJob unused, so it is removed.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V2:
Don't acquire a job in libxlDomainAutoCoreDump since caller already
has a job.
Fix typos in commit message.
src/libxl/libxl_domain.c | 53
src/libxl/libxl_domain.h | 4
src/libxl/libxl_driver.c | 20 ++
3 files changed, 29 insertions(+), 48 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 6b9959f..faf2c19 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -371,6 +371,9 @@ libxlDomainShutdownThread(void *opaque)
cfg = libxlDriverConfigGet(driver);
+if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0)
+goto cleanup;
+
if (xl_reason == LIBXL_SHUTDOWN_REASON_POWEROFF) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
@@ -384,7 +387,7 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_PRESERVE:
case VIR_DOMAIN_LIFECYCLE_LAST:
-goto cleanup;
+goto endjob;
}
} else if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
dom_event = virDomainEventLifecycleNewFromObj(vm,
@@ -399,7 +402,7 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE:
case VIR_DOMAIN_LIFECYCLE_CRASH_LAST:
-goto cleanup;
+goto endjob;
case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY:
libxlDomainAutoCoreDump(driver, vm);
goto destroy;
@@ -420,11 +423,11 @@ libxlDomainShutdownThread(void *opaque)
goto restart;
case VIR_DOMAIN_LIFECYCLE_PRESERVE:
case VIR_DOMAIN_LIFECYCLE_LAST:
-goto cleanup;
+goto endjob;
}
} else {
VIR_INFO(Unhandled shutdown_reason %d, xl_reason);
-goto cleanup;
+goto endjob;
}
destroy:
@@ -433,13 +436,11 @@ libxlDomainShutdownThread(void *opaque)
dom_event = NULL;
}
libxl_domain_destroy(cfg-ctx, vm-def-id, NULL);
-if (libxlDomainCleanupJob(driver, vm, reason)) {
-if (!vm-persistent) {
-virDomainObjListRemove(driver-domains, vm);
-vm = NULL;
-}
-}
-goto cleanup;
+libxlDomainCleanup(driver, vm, reason);
+if (!vm-persistent)
+virDomainObjListRemove(driver-domains, vm);
+
+goto endjob;
restart:
if (dom_event) {
@@ -447,13 +448,17 @@ libxlDomainShutdownThread(void *opaque)
dom_event = NULL;
}
libxl_domain_destroy(cfg-ctx, vm-def-id, NULL);
-libxlDomainCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
if (libxlDomainStart(driver, vm, false, -1) 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_(Failed to restart VM '%s': %s),
vm-def-name, err ? err-message : _(unknown error));
}
+ endjob:
+if (!libxlDomainObjEndJob(driver, vm))
+vm = NULL;
+
cleanup:
if (vm)
virObjectUnlock(vm);
@@ -691,26 +696,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
}
/*
- * Cleanup function for domain that has reached shutoff state.
- * Executed in the context of a job.
- *
- * virDomainObjPtr should be locked on invocation
- * Returns true if references remain on virDomainObjPtr, false otherwise.
- */
-bool
-libxlDomainCleanupJob(libxlDriverPrivatePtr driver,
- virDomainObjPtr vm,
- virDomainShutoffReason reason)
-{
-if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_DESTROY) 0)
-return true;
-
-libxlDomainCleanup(driver, vm, reason);
-
-return libxlDomainObjEndJob(driver, vm);
-}
-
-/*
* Core dump domain to default dump path.
*
* virDomainObjPtr must be locked on invocation
@@ -735,15 +720,11 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
timestr) 0)
goto cleanup;
-if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0)
-goto cleanup;
-
/* Unlock virDomainObj while dumping core */
virObjectUnlock(vm);
libxl_domain_core_dump(cfg-ctx, vm-def-id, dumpfile, NULL);
virObjectLock(vm);
-ignore_value(libxlDomainObjEndJob(driver, vm));
ret = 0;
cleanup:
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index a032e46..30855a2 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -108,10 +108,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
[libvirt] [PATCH V2 0/3] libxl: domain destroy fixes
V2 of a small series to fix issues wrt domain destroy https://www.redhat.com/archives/libvir-list/2015-March/msg01337.html Comments from Konrad and Martin are addressed in this version. 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 src/libxl/libxl_domain.c | 81 ++-- src/libxl/libxl_domain.h | 4 --- src/libxl/libxl_driver.c | 80 +++ 3 files changed, 91 insertions(+), 74 deletions(-) -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On Wed, Apr 01, 2015 at 09:05:31PM +0200, Michael Mueller wrote: On Wed, 1 Apr 2015 13:59:05 -0300 Eduardo Habkost ehabk...@redhat.com wrote: Not directly invalid as -cpu none will be the same as omitting the -cpu option. KVM will setup the vcpu properties withou any QEMU control to whatever the hosting machine and the kvm kernel code offers. That will allow to run QEMU against a KVM version that is not aware of the s390 cpu model ioctls. It looks like we have conflicting expectations about query-cpu-definitions, it seems: on the one hand, if -cpu none is valid I believe it should appear on the query-cpu-definitions return value; on the other hand, it is not (always?) migration-safe, so just comparing the source query-cpus data with the target query-cpu-definitions data wouldn't be enough to ensure live-migration safety. There are other cases as well where the value given with -cpu is *not* part of the cpu definition list and that is aliases: [mimu@p57lp59 (master-cpu-model) qemu]$ ./s390x-softmmu/qemu-system-s390x -machine s390-ccw,accel=kvm -cpu ? s390 2064-ga1 IBM zSeries 900 GA1 s390 2064-ga2 IBM zSeries 900 GA2 s390 2064-ga3 IBM zSeries 900 GA3 s390 2064 (alias for 2064-ga3) s390 z900 (alias for 2064-ga3) ... s390 z10(alias for 2097-ga3) s390 z10-ec (alias for 2097-ga3) s390 2098-ga1 IBM System z10 BC GA1 s390 2098-ga2 IBM System z10 BC GA2 s390 2098 (alias for 2098-ga2) s390 z10-bc (alias for 2098-ga2) s390 2817-ga1 IBM zEnterprise 196 GA1 s390 2817-ga2 IBM zEnterprise 196 GA2 s390 2817 (alias for 2817-ga2) s390 z196 (alias for 2817-ga2) s390 2818-ga1 IBM zEnterprise 114 GA1 s390 2818 (alias for 2818-ga1) s390 z114 (alias for 2818-ga1) s390 2827-ga1 IBM zEnterprise EC12 GA1 s390 2827-ga2 IBM zEnterprise EC12 GA2 s390 2827 (alias for 2827-ga2) s390 zEC12 (alias for 2827-ga2) s390 host (alias for 2827-ga2) s390 2828-ga1 IBM zEnterprise BC12 GA1 s390 2828 (alias for 2828-ga1) s390 zBC12 (alias for 2828-ga1) As you can see host is in s390x case always an alias and also all other aliases are normalized to their real cpu models in the cpu-definitions list. On x86, we have a similar problem with -cpu host, that changes depending on the host hardware and host kernel. We solve this problem by making libvirt code aware of the set of valid CPU models, and libvirt has special cases for -cpu host. -cpu host is not a special case for s390, it will return (2827-ga2, kvm) as cpu model or whatever model the hosting system implements. If you don't want to encode that knowledge in libvirt or other management software for s390, it looks like you need something like a stable-abi-safe field on CpuDefinitionInfo? Exactly that fulfills the name field for s390 already in my view. And cpu model none just means that QEMU does not manage the cpu model. That's also the reason why I initially returned an empty [] model and not none. This somewhat convinces me to go back to this approach... I understand the reasons for your approach and it seems to work for s390, but the only problem I see is that you are adding an additional (undocumented?) s390-specific constraint to the semantics of query-cpu-models: that the model name will appear on the list only if it can be safely migratable. This may prevent us from unifying CPU model code into generic code later. But if we add a simple stable-abi-safe field to the list (even if s390 set it to to true for all models and omit aliases and none in this first version), we will have clearer semantics that can still be honoured by other architectures (and by generic code) later. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
