[libvirt] [PATCHv4 0/2] vnc/spice listen address as a network name
This is the latest reincarnation of this patch series: https://www.redhat.com/archives/libvir-list/2011-July/msg01717.html In the previous version, I had created a new subelement of graphics, called listen, and placed all listen address and port attributes there. After discussion on the list and in IRC with Dan Berrange and Eric Blake, I became convinced that was overdoing things, and that really only the address or network name should be in the listen element, but that port/tlsPort/autoport should stay put. This version removes all changes to port-related attributes, paring it down to just the listen address (and the new listen network) attribute. This greatly simplifies the patch, so new review of it should be easier. Based on Eric's review, the nwe version tries to be more concise about what is accepted, what is ignored, and what is rejected in the XML, both by the parser and by the RNG. (since there is less to deal with, this is also easier.) Having a single port value but multiple addresses does create some potential future restrictions, which we can deal with an eliminate later if they become problematic. In particular, each graphics device will have only a single port, but could have multiple listen addresses; in this case, all the listen addresses would have to listen on the same port. (This isn't a problem right now, because all of our drivers can only listen on a single address anyway). (Note to Eric: I did consider putting the switch to helper functions into its own patch, but after removing all the ports from listen, the number of helper functions (and thus the number of changes created by switching to using them, has been reduced considerably, so the gain wouldn't be as much. (also, it's very late, and I'd like to get some sleep tonight :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 2/2] qemu: support type=network in domain graphics listen
The domain XML now understands the listen subelement of its
graphics element (including when listen type='network'), and the
network driver has an internal API that will turn a network name into
an IP address, so the final logical step is to put the glue into the
qemu driver so that when it is starting up a domain, if it finds
listen type='network' network='xyz'/ in the XML, it will call the
network driver to get an IPv4 address associated with network xyz, and
tell qemu to listen for vnc (or spice) on that address rather than the
default address (localhost).
The motivation for this is that a large installation may want the
guests' VNC servers listening on physical interfaces rather than
localhost, so that users can connect directly from the outside; this
requires sending qemu the appropriate IP address to listen on. But
this address will of course be different for each host, and if a guest
might be migrated around from one host to another, it's important that
the guest's config not have any information embedded in it that is
specific to one particular host. listen type='network.../ can solve
this problem in the following manner:
1) on each host, define a libvirt network of the same name,
associated with the interface on that host that should be used
for listening (for example, a simple macvtap network: forward
mode='bridge' dev='eth0'/, or host bridge network: forward
mode='bridge'/ bridge name='br0'/
2) in the graphics element of each guest's domain xml, tell vnc to
listen on the network name used in step 1:
graphics type='vnc' port='5922'
listen type='network'network='example-net'/
/graphics
(all the above also applies for graphics type='spice').
---
src/qemu/qemu_command.c | 73 +-
src/qemu/qemu_hotplug.c | 13
2 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 740468f..ee42f1d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4127,10 +4127,43 @@ qemuBuildCommandLine(virConnectPtr conn,
def-graphics[0]-data.vnc.socket);
} else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
+const char *listenNetwork;
const char *listenAddr = NULL;
+char *netAddr = NULL;
bool escapeAddr;
+int ret;
+
+switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) {
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
+listenAddr =
virDomainGraphicsListenGetAddress(def-graphics[0], 0);
+break;
+
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
+listenNetwork =
virDomainGraphicsListenGetNetwork(def-graphics[0], 0);
+if (!listenNetwork)
+break;
+ret = networkGetNetworkAddress(listenNetwork, netAddr);
+if (ret = -2) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+%s, _(network-based listen not
possible,
+network driver not present));
+goto error;
+}
+if (ret 0) {
+qemuReportError(VIR_ERR_XML_ERROR,
+_(listen network '%s' had no usable
address),
+listenNetwork);
+goto error;
+}
+listenAddr = netAddr;
+/* store the address we found in the graphics element so it
will
+ * show up in status. */
+if (virDomainGraphicsListenSetAddress(def-graphics[0], 0,
+ listenAddr, -1, false)
0)
+ goto error;
+break;
+}
-listenAddr = virDomainGraphicsListenGetAddress(def-graphics[0],
0);
if (!listenAddr)
listenAddr = driver-vncListen;
@@ -4142,6 +4175,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(opt, :%d,
def-graphics[0]-data.vnc.port - 5900);
+VIR_FREE(netAddr);
} else {
virBufferAsprintf(opt, %d,
def-graphics[0]-data.vnc.port - 5900);
@@ -4225,7 +4259,10 @@ qemuBuildCommandLine(virConnectPtr conn,
} else if ((def-ngraphics == 1)
def-graphics[0]-type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
virBuffer opt = VIR_BUFFER_INITIALIZER;
+const char *listenNetwork;
const char *listenAddr = NULL;
+char *netAddr = NULL;
+int ret;
if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SPICE)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -4238,12 +4275,44 @@ qemuBuildCommandLine(virConnectPtr conn,
if (driver-spiceTLS
Re: [libvirt] [PATCH 00/19] Rollback migration when libvirtd restarts
At 07/28/2011 05:41 AM, Eric Blake Write:
On 07/07/2011 05:34 PM, Jiri Denemark wrote:
This series is also available at
https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery
The series does several things:
- persists current job and its phase in status xml
- allows safe monitor commands to be run during migration/save/dump jobs
- implements recovery when libvirtd is restarted while a job is active
- consolidates some code and fixes bugs I found when working in the area
git bisect is pointing to this series as the cause of a regression in
'virsh managedsave dom' triggering libvirtd core dumps if some other
process is actively making queries on domain at the same time
(virt-manager is a great process for fitting that bill). I'm trying to
further narrow down which patch introduced the regression, and see if I
can plug the race (probably a case of not checking whether the monitor
still exists when getting the condition for an asynchronous job, since
the whole point of virsh [managed]save is that the domain will go away
when the save completes, but that it is time-consuming enough that we
want to query domain state in the meantime).
I can reproduce this bug.
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x706d0700 (LWP 11419)]
0x004b9ad8 in qemuMonitorSend (mon=0x7fffe815c060,
msg=0x706cf380)
at qemu/qemu_monitor.c:801
801while (!mon-msg-finished) {
The reason is that mon-msg is NULL.
I add some debug codes, and found that we send monitor command while
the last command is not finished, and then libvirtd crashed.
After reading the code, I think something is wrong in the function
qemuDomainObjEnterMonitorInternal():
if (priv-job.active == QEMU_JOB_NONE priv-job.asyncJob) {
if (qemuDomainObjBeginNestedJob(driver, obj) 0)
We can run query job while asyncJob is running. When we query the migration's
status, priv-job.active is not QEMU_JOB_NONE, and we do not wait the query job
finished. So we send monitor command while last command is not finished. It's
very
dangerous.
When we run a async job, we can not know whether the job is nested async job
according
to priv-job.active's value.
I think we should introduce four functions for async nested job:
qemuDomainObjAsyncEnterMonitor()
qemuDomainObjAsyncEnterMonitorWithDriver()
qemuDomainObjAsyncExitMonitor()
qemuDomainObjAsyncExitMonitorWithDriver()
The qemuDomainObjEnterMonitorInternal()'s caller should pass a bool value to
tell
qemuDomainObjEnterMonitorInternal() whether the job is a async nested job.
Thanks
Wen Congyang.
(gdb) bt
#0 0x004b9ad8 in qemuMonitorSend (mon=0x7fffe815c060,
msg=0x706cf380) at qemu/qemu_monitor.c:801
#1 0x004c77ae in qemuMonitorJSONCommandWithFd (mon=0x7fffe815c060,
cmd=0x7fffd8000940, scm_fd=-1, reply=0x706cf480)
at qemu/qemu_monitor_json.c:225
#2 0x004c78e5 in qemuMonitorJSONCommand (mon=0x7fffe815c060,
cmd=0x7fffd8000940, reply=0x706cf480) at
qemu/qemu_monitor_json.c:254
#3 0x004cc19c in qemuMonitorJSONGetMigrationStatus (
mon=0x7fffe815c060, status=0x706cf580, transferred=0x706cf570,
remaining=0x706cf568, total=0x706cf560)
at qemu/qemu_monitor_json.c:1920
#4 0x004bc1b3 in qemuMonitorGetMigrationStatus
(mon=0x7fffe815c060,
status=0x706cf580, transferred=0x706cf570,
remaining=0x706cf568, total=0x706cf560) at
qemu/qemu_monitor.c:1532
#5 0x004b201b in qemuMigrationUpdateJobStatus
(driver=0x7fffe80089f0,
vm=0x7fffe8015cd0, job=0x5427b6 domain save job)
at qemu/qemu_migration.c:765
#6 0x004b2383 in qemuMigrationWaitForCompletion (
driver=0x7fffe80089f0, vm=0x7fffe8015cd0) at qemu/qemu_migration.c:846
#7 0x004b7806 in qemuMigrationToFile (driver=0x7fffe80089f0,
vm=0x7fffe8015cd0, fd=27, offset=4096,
path=0x7fffd8000990 /var/lib/libvirt/qemu/save/fedora_12.save,
compressor=0x0, is_reg=true, bypassSecurityDriver=true)
at qemu/qemu_migration.c:2766
#8 0x0046a90d in qemuDomainSaveInternal (driver=0x7fffe80089f0,
dom=0x7fffd8000ad0, vm=0x7fffe8015cd0,
path=0x7fffd8000990 /var/lib/libvirt/qemu/save/fedora_12.save,
compressed=0, bypass_cache=false) at qemu/qemu_driver.c:2386
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] python: add python binding for virDomainGetBlkioParameters
On 27.07.2011 04:13, Hu Tao wrote: --- python/libvirt-override-api.xml |6 +- python/libvirt-override.c | 79 +- 2 files changed, 79 insertions(+), 6 deletions(-) ACK It actually exposed we need this update: diff --git a/src/libvirt.c b/src/libvirt.c index eeaf0b6..2ce391c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3516,7 +3516,7 @@ error: * @params: pointer to blkio parameter object * (return value, allocated by the caller) * @nparams: pointer to number of blkio parameters - * @flags: currently unused, for future extension + * @flags: an OR'ed set of virDomainModificationImpact * * Get all blkio parameters, the @params array will be filled with the values * equal to the number of parameters suggested by @nparams. But that will better be a follow up patch. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] python: add python binding for virDomainSetBlkioParameters
On Thu, Jul 28, 2011 at 10:15:21AM +0200, Michal Privoznik wrote: On 27.07.2011 04:13, Hu Tao wrote: --- python/libvirt-override-api.xml |1 + python/libvirt-override.c | 94 +- 2 files changed, 92 insertions(+), 3 deletions(-) ACK, but again, we need update of virDomainSetBlkioParameters description. I'll collect all of these and push it then as one patch. Thanks. -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] python: add python binding for virDomainGetMemoryParameters
On 27.07.2011 04:13, Hu Tao wrote: --- python/libvirt-override-api.xml |6 +- python/libvirt-override.c | 79 +- 2 files changed, 79 insertions(+), 6 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] python: add python binding for virDomainSetBlkioParameters
On 27.07.2011 04:13, Hu Tao wrote: --- python/libvirt-override-api.xml |1 + python/libvirt-override.c | 94 +- 2 files changed, 92 insertions(+), 3 deletions(-) ACK, but again, we need update of virDomainSetBlkioParameters description. I'll collect all of these and push it then as one patch. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/19] Rollback migration when libvirtd restarts
At 07/28/2011 03:26 PM, Wen Congyang Write:
At 07/28/2011 05:41 AM, Eric Blake Write:
On 07/07/2011 05:34 PM, Jiri Denemark wrote:
This series is also available at
https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery
The series does several things:
- persists current job and its phase in status xml
- allows safe monitor commands to be run during migration/save/dump jobs
- implements recovery when libvirtd is restarted while a job is active
- consolidates some code and fixes bugs I found when working in the area
git bisect is pointing to this series as the cause of a regression in
'virsh managedsave dom' triggering libvirtd core dumps if some other
process is actively making queries on domain at the same time
(virt-manager is a great process for fitting that bill). I'm trying to
further narrow down which patch introduced the regression, and see if I
can plug the race (probably a case of not checking whether the monitor
still exists when getting the condition for an asynchronous job, since
the whole point of virsh [managed]save is that the domain will go away
when the save completes, but that it is time-consuming enough that we
want to query domain state in the meantime).
I can reproduce this bug.
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x706d0700 (LWP 11419)]
0x004b9ad8 in qemuMonitorSend (mon=0x7fffe815c060,
msg=0x706cf380)
at qemu/qemu_monitor.c:801
801while (!mon-msg-finished) {
The reason is that mon-msg is NULL.
I add some debug codes, and found that we send monitor command while
the last command is not finished, and then libvirtd crashed.
After reading the code, I think something is wrong in the function
qemuDomainObjEnterMonitorInternal():
if (priv-job.active == QEMU_JOB_NONE priv-job.asyncJob) {
if (qemuDomainObjBeginNestedJob(driver, obj) 0)
We can run query job while asyncJob is running. When we query the migration's
status, priv-job.active is not QEMU_JOB_NONE, and we do not wait the query
job
finished. So we send monitor command while last command is not finished. It's
very
dangerous.
When we run a async job, we can not know whether the job is nested async job
according
to priv-job.active's value.
I think we should introduce four functions for async nested job:
Some functions(for example qemuProcessStopCPUs) can be used by sync job and
async job.
So we do not know which type job call these functions when we enter these
functions.
We support run sync job and async job at the same time. It means that the
monitor commands
for two jobs can be run in any order.
Another way to fix this bug is:
If we try to send monitor command while the last command is not finished, we
wait the last monitor command
to finish.
qemuDomainObjAsyncEnterMonitor()
qemuDomainObjAsyncEnterMonitorWithDriver()
qemuDomainObjAsyncExitMonitor()
qemuDomainObjAsyncExitMonitorWithDriver()
The qemuDomainObjEnterMonitorInternal()'s caller should pass a bool value to
tell
qemuDomainObjEnterMonitorInternal() whether the job is a async nested job.
Thanks
Wen Congyang.
(gdb) bt
#0 0x004b9ad8 in qemuMonitorSend (mon=0x7fffe815c060,
msg=0x706cf380) at qemu/qemu_monitor.c:801
#1 0x004c77ae in qemuMonitorJSONCommandWithFd (mon=0x7fffe815c060,
cmd=0x7fffd8000940, scm_fd=-1, reply=0x706cf480)
at qemu/qemu_monitor_json.c:225
#2 0x004c78e5 in qemuMonitorJSONCommand (mon=0x7fffe815c060,
cmd=0x7fffd8000940, reply=0x706cf480) at
qemu/qemu_monitor_json.c:254
#3 0x004cc19c in qemuMonitorJSONGetMigrationStatus (
mon=0x7fffe815c060, status=0x706cf580, transferred=0x706cf570,
remaining=0x706cf568, total=0x706cf560)
at qemu/qemu_monitor_json.c:1920
#4 0x004bc1b3 in qemuMonitorGetMigrationStatus
(mon=0x7fffe815c060,
status=0x706cf580, transferred=0x706cf570,
remaining=0x706cf568, total=0x706cf560) at
qemu/qemu_monitor.c:1532
#5 0x004b201b in qemuMigrationUpdateJobStatus
(driver=0x7fffe80089f0,
vm=0x7fffe8015cd0, job=0x5427b6 domain save job)
at qemu/qemu_migration.c:765
#6 0x004b2383 in qemuMigrationWaitForCompletion (
driver=0x7fffe80089f0, vm=0x7fffe8015cd0) at qemu/qemu_migration.c:846
#7 0x004b7806 in qemuMigrationToFile (driver=0x7fffe80089f0,
vm=0x7fffe8015cd0, fd=27, offset=4096,
path=0x7fffd8000990 /var/lib/libvirt/qemu/save/fedora_12.save,
compressor=0x0, is_reg=true, bypassSecurityDriver=true)
at qemu/qemu_migration.c:2766
#8 0x0046a90d in qemuDomainSaveInternal (driver=0x7fffe80089f0,
dom=0x7fffd8000ad0, vm=0x7fffe8015cd0,
path=0x7fffd8000990 /var/lib/libvirt/qemu/save/fedora_12.save,
compressed=0, bypass_cache=false) at qemu/qemu_driver.c:2386
--
libvir-list mailing list
libvir-list@redhat.com
Re: [libvirt] [PATCH 4/4] python: add python binding for virDomainSetMemoryParameters
On 27.07.2011 04:13, Hu Tao wrote: --- python/libvirt-override-api.xml |1 + python/libvirt-override.c | 94 +- 2 files changed, 92 insertions(+), 3 deletions(-) ACK. I pushed the whole set and update of flags description for virDomainSetBlkioParameters and virDomainGetBlkioParameters. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: avoid missing zero value judgement in cmdBlkiotune
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to
VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jia a...@redhat.com
---
tools/virsh.c | 10 --
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..183d7c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,12 +4037,10 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
-vshError(ctl, _(Invalid value of %d for I/O weight), weight);
-goto cleanup;
-}
+nparams++;
+if (weight = 0) {
+vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+goto cleanup;
}
if (nparams == 0) {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] virsh: avoid missing zero value judgement in cmdBlkiotune
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jia a...@redhat.com
---
tools/virsh.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..512f2c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,14 +4037,13 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
-vshError(ctl, _(Invalid value of %d for I/O weight), weight);
-goto cleanup;
-}
+if (weight = 0) {
+vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+goto cleanup;
}
+nparams++;
+
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL, nparams, flags) != 0) {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: API additions for enhanced snapshot support
On Wed, Jul 27, 2011 at 3:17 PM, Eric Blake ebl...@redhat.com wrote: On 07/27/2011 04:04 AM, Stefan Hajnoczi wrote: On Thu, Jun 16, 2011 at 6:41 AM, Eric Blakeebl...@redhat.com wrote: Right now, libvirt has a snapshot API via virDomainSnapshotCreateXML, but for qemu domains, it only works if all the guest disk images are qcow2, and qemu rather than libvirt does all the work. However, it has a couple of drawbacks: it is inherently tied to domains (there is no way to manage snapshots of storage volumes not tied to a domain, even though libvirt does that for qcow2 images associated with offline qemu domains by using the qemu-img application). And it necessarily operates on all of the images associated with a domain in parallel - if any disk image is not qcow2, the snapshot fails, and there is no way to select a subset of disks to save. However, it works on both active (disk and memory state) and inactive domains (just disk state). Hi Eric, Any updates on your proposed snapshot API enhancements? I still need to post a v2 RFC that gives the XML changes needed to support disk snapshots on top of virDomainSnapshotCreateXML. Meanwhile, I still want to add additional API to make it easier to manage offline storage volume snapshots (storage volumes that are not in use by a defined or running domain), although it obviously won't happen by 0.9.4. Previous discussion has focussed on image files. Is part of your plan to extend virStorageBackend and let the LVM backend support the new APIs? The use case I am particularly interested in is a backup solution using libvirt snapshot APIs to take consistent backups of guests. The two workflows are reading out full snapshots of disk images (full backup) and reading only those blocks that have changed since the last backup (incremental backup). Full backup should be supported via virDomainSnapshotCreateXML, once I have the new XML in place, by using the new qemu snapshot_blkdev monitor commands. Excellent :) Incremental backups can be done just like full backups except with an API call to get a dirty blocks list. The client only reads out those dirty blocks from the snapshot. Incremental backups will need more work - qemu does not yet have the monitor commands for exposing which blocks are dirty. Of course, as code is written for qemu, it would be nice to also be thinking about how to support that in libvirt, so once I do propose my XML changes for full snapshots, it would be nice to remember in your review to think about whether it remains extensible to the incremental case. Robert Wang and I would like to help make the incremental use case possible. Like you say, QEMU does not have this feature yet but it makes sense to plan together with libvirt. Will keep you CCed on QEMU discussions about adding this feature. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: libvirt-0.9.1 to 0.9.3-r1: managedsave/save won't start/restore at saved state
The 27/07/11, Laine Stump wrote: If you pause (virsh suspend) the guest before the save, can you then successfully start the domain (it should start in a paused state, and when the virsh commandline returns to the prompt, you can resume it with virsh resume.) If this is the case, it could be a race condition within qemu. This is why I like help from other: I would never had the idea to do this check on my own. Unfortunately, this didn't help: root@homer uptime 11:05:34 up 8 min, 1 user, load average: 0.00, 0.00, 0.00 root@homer virsh # suspend homer Domain homer suspended virsh # list Id Name State -- 21 homerpaused virsh # managedsave homer Domain homer state saved by libvirt virsh # list Id Name State -- virsh # start homer Domain homer started virsh # list Id Name State -- 22 homerpaused virsh # resume homer Domain homer resumed virsh # root@homer uptime 11:06:51 up 0 min, 1 user, load average: 0.00, 0.00, 0.00 root@homer Thank you. -- Nicolas Sebrecht -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: avoid missing zero value judgement in cmdBlkiotune
On 07/28/2011 05:16 PM, Michal Privoznik wrote:
On 28.07.2011 11:04, Alex Jia wrote:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune,
when
weight is equal to 0, the cmdBlkiotune will not raise any error
information
when judge weight value first time, and execute else branch to
judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if
param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying
qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
I liked the first verion more than this. Although it is not shown in
this snippet,
nparams is initialized to zero
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..512f2c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,14 +4037,13 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
-vshError(ctl, _(Invalid value of %d for I/O weight),
weight);
-goto cleanup;
-}
+if (weight= 0) {
+vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+goto cleanup;
}
and therefore doing this
+nparams++;
+
will invalidate this test.
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL,nparams, flags)
!= 0) {
The better way of dealing with this issue IMO is to switch from
if (weight) {
to test if 'weight' parameter was given (vshCommandOptInt() returns
0) and after this
check for impermissible values. And after the latter test (still
inside 'weight argument
given') increment nparams:
if (weight_given) {
if (weight=0) {
report error; goto cleanup;
nparams++;
}
Michal
I aware of this , absolutely agree, thanks for your nice comment.
Regards,
Alex
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: libvirt-0.9.1 to 0.9.3-r1: managedsave/save won't start/restore at saved state
The 27/07/11, Eric Blake wrote: On 07/27/2011 02:37 AM, Nicolas Sebrecht wrote: I'm seeing strange behaviour, here. Any guests saved using both managedsave and save commands from virsh won't restore at saved state. A new full boot sequence happen. - Tested against libvirt v0.9.1, v0.9.2, v0.9.3-r1 (Gentoo) - Confirmed on three different hosts Gentoo amd64 systems. - Tested with gentoo and ubuntu guests. - Nothing relevant in /var/log/libvirt/libvirt.log or /var/log/libvirt/qemu/dom.log The state file /var/lib/libvirt/qemu/save/dom.save exists and is deleted when 'virsh start' is called. The new boot sequence is confirmed by : - VNC console checks - previous screen sessions lost - uptime I've open a bug at https://bugs.gentoo.org/show_bug.cgi?id=376333 but had no answer. Any idea on what could happen or how to inspect it? Does /var/log/libvirt/qemu/dom.log show the qemu process getting started with the -incoming fd:nnn flag? While you claim that nothing appeared to be relevant in that log, it might actually help to post a few lines of it for confirmation. Here is a fresh test. Hostnames are: nicolas-desktop: my desktop homer: guest (logged as root) xenon: host (logged as root) nicolas@nicolas-desktop ssh homer.test.lan root@homer uptime 10:06:44 up 3 min, 1 user, load average: 0.10, 0.24, 0.11 root@homer exit nicolas@nicolas-desktop ssh xenon.test.lan xenon ~ # virsh managedsave homer Domain homer state saved by libvirt xenon ~ # cd /var/lib/libvirt/qemu/save xenon save # ls -l total 195M -rw--- 1 root root 195M Jul 28 10:08 homer.save waiting a bit xenon save # virsh start homer Domain homer started xenon save # ls -l total 0 xenon save # exit nicolas@nicolas-desktop ssh homer.test.lan root@homer uptime 10:22:42 up 0 min, 1 user, load average: 0.00, 0.00, 0.00 root@homer nicolas@nicolas-desktop ssh xenon.test.lan xenon ~ # tail /var/log/libvirt/qemu/homer.log 2011-07-28 10:03:07.718: shutting down 2011-07-28 10:03:41.103: starting up LC_ALL=C PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.4.5:/root/bin HOME=/root USER=root QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc-0.11 -enable-kvm -m 512 -smp 2,sockets=2,cores=1,threads=1 -name homer -uuid 90b87fd0-6add-c7c8-e6f8-b8245bae8329 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/homer.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -drive file=/home/piing/libvirt/images/piing/homer.img,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,bus=pci.0,multifunction=on,addr=0x5.0x0,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/root/virtuals/images/piing/homer-lun0.raw,if=none,id=drive-virtio-disk1,format=raw -device virtio-blk-pci,bus=pci.0,multifunction=on,addr=0x8.0x0,drive=drive-virtio-disk1,id=virtio-disk1 -drive if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -devi! ce ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -netdev tap,fd=17,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:c3:7b:da,bus=pci.0,multifunction=on,addr=0x4.0x0 -netdev tap,fd=18,id=hostnet1 -device virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:30:15:24,bus=pci.0,multifunction=on,addr=0x3.0x0 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -vga cirrus -incoming fd:13 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x6.0x0 Domain id=19 is tainted: high-privileges char device redirected to /dev/pts/4 2011-07-28 10:08:11.024: shutting down 2011-07-28 10:22:48.203: starting up LC_ALL=C PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.4.5:/root/bin HOME=/root USER=root QEMU_AUDIO_DRV=none /usr/bin/qemu-kvm -S -M pc-0.11 -enable-kvm -m 512 -smp 2,sockets=2,cores=1,threads=1 -name homer -uuid 90b87fd0-6add-c7c8-e6f8-b8245bae8329 -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/homer.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -drive file=/home/piing/libvirt/images/piing/homer.img,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,bus=pci.0,multifunction=on,addr=0x5.0x0,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/root/virtuals/images/piing/homer-lun0.raw,if=none,id=drive-virtio-disk1,format=raw -device virtio-blk-pci,bus=pci.0,multifunction=on,addr=0x8.0x0,drive=drive-virtio-disk1,id=virtio-disk1 -drive if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -devi! ce ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -netdev tap,fd=18,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:c3:7b:da,bus=pci.0,multifunction=on,addr=0x4.0x0 -netdev tap,fd=20,id=hostnet1 -device
[libvirt] [PATCH v3] virsh: avoid missing zero value judgement in cmdBlkiotune
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jia a...@redhat.com
---
tools/virsh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..f24050d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,12 +4037,12 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
+if (vshCommandOptInt(cmd, weight, weight) 0) {
+if (weight = 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
+nparams++;
}
if (nparams == 0) {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] virsh: avoid missing zero value judgement in cmdBlkiotune
At 07/28/2011 05:52 PM, Alex Jia Write:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jia a...@redhat.com
---
tools/virsh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..f24050d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,12 +4037,12 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
+if (vshCommandOptInt(cmd, weight, weight) 0) {
Why you call vshCommandOptInt(cmd, weight, weight) again?
+if (weight = 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
+nparams++;
}
if (nparams == 0) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: avoid missing zero value judgement in cmdBlkiotune
On 28.07.2011 11:26, Alex Jia wrote:
On 07/28/2011 05:04 PM, Alex Jia wrote:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error
information
when judge weight value first time, and execute else branch to judge
weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is
equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters
function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..512f2c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,14 +4037,13 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
cmd)
goto cleanup;
}
- if (weight) {
- nparams++;
- if (weight 0) {
- vshError(ctl, _(Invalid value of %d for I/O weight), weight);
- goto cleanup;
- }
+ if (weight= 0) {
+ vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+ goto cleanup;
}
+ nparams++;
+
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL,nparams, flags) != 0) {
Hmm, the above patch will introduce new issue, if blkiotune without any
weight option,
vshError will also be hit, that is not we expected.
The reason of root is vshCommandOptInt will assign 0 to weight if option
not found
and not required, so we need to specifically deal with 0 value, I will
renew consider this
issue, so cancel this patch.
Actually, vshCommandOptInt (and all virsh opts parsing functions), touches
given variable only iff found valid. In all other cases the variable
remains untouched. It's the initialization which set weight to 0.
The reason for such return values is, we can firstly initialize variable,
then call option parsing function over it which will change it iff
everything's fine:
int var = 10;
if (vshCommandOptInt(cmd, int-attr, var) 0) {
/* error, e.g. string value given */
goto cleanup;
}
/* here, var may be 10, iff int-attr was not given,
or other value if it was */
Or, if int-att is required, we just change the test:
if (vshCommandOptInt(cmd, int-attr, var) = 0) {
Perhaps this will help :)
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid type-punning compiler warning
2011/7/27 Eric Blake ebl...@redhat.com: On RHEL 5, with gcc 4.1.2: rpc/virnetsaslcontext.c: In function 'virNetSASLSessionUpdateBufSize': rpc/virnetsaslcontext.c:396: warning: dereferncing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] * src/rpc/virnetsaslcontext.c (virNetSASLSessionUpdateBufSize): Use a union to work around gcc warning. --- src/rpc/virnetsaslcontext.c | 11 +++ 1 files changed, 7 insertions(+), 4 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] support qemuMonitorSend() to be called at the same time
Current, we support run sync job and async job at the same time. It means that
the
monitor commands for two jobs can be run in any order.
In the function qemuDomainObjEnterMonitorInternal():
if (priv-job.active == QEMU_JOB_NONE priv-job.asyncJob) {
if (qemuDomainObjBeginNestedJob(driver, obj) 0)
We check whether the caller is an async job by priv-job.active and
priv-job.asynJob. But when an async job is running, priv-job.active
is QEMU_JOB_NONE if no sync job is running, or priv-job.active is not
QEMU_JOB_NONE if a sync job is running. So we cannot check whether the
caller is an async job in the function qemuDomainObjEnterMonitorInternal().
Unfortunately, if sync job and async job are running at the same time, we may
try to send monitor command at the same time in two threads. It's very
dangerous, and it will cause libvirtd crashed.
We should enhance the function qemuMonitorSend() to support it to be called
at the same time.
If the last monitor command does not finish, the other monitor commands should
wait it to finish.
---
src/qemu/qemu_monitor.c | 22 +-
1 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index db6107c..a10f53f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -46,6 +46,8 @@ struct _qemuMonitor {
virMutex lock; /* also used to protect fd */
virCond notify;
+virCond send_notify;
+
int refs;
int fd;
@@ -675,7 +677,8 @@ qemuMonitorOpen(virDomainObjPtr vm,
VIR_FREE(mon);
return NULL;
}
-if (virCondInit(mon-notify) 0) {
+if ((virCondInit(mon-notify) 0) ||
+(virCondInit(mon-send_notify) 0)) {
qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cannot initialize monitor condition));
virMutexDestroy(mon-lock);
@@ -795,6 +798,22 @@ int qemuMonitorSend(qemuMonitorPtr mon,
return -1;
}
+while (mon-msg) {
+if (virCondWait(mon-send_notify, mon-lock) 0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(Unable to wait on monitor condition));
+goto cleanup;
+}
+}
+
+/* Check whether qemu quited unexpectedly again */
+if (mon-lastError.code != VIR_ERR_OK) {
+VIR_DEBUG(Attempt to send command while error is set %s,
+ NULLSTR(mon-lastError.message));
+virSetError(mon-lastError);
+return -1;
+}
+
mon-msg = msg;
qemuMonitorUpdateWatch(mon);
@@ -818,6 +837,7 @@ int qemuMonitorSend(qemuMonitorPtr mon,
cleanup:
mon-msg = NULL;
qemuMonitorUpdateWatch(mon);
+virCondBroadcast(mon-send_notify);
return ret;
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: avoid missing zero value judgement in cmdBlkiotune
On 07/28/2011 05:55 PM, Michal Privoznik wrote:
On 28.07.2011 11:26, Alex Jia wrote:
On 07/28/2011 05:04 PM, Alex Jia wrote:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune,
when
weight is equal to 0, the cmdBlkiotune will not raise any error
information
when judge weight value first time, and execute else branch to judge
weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is
equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters
function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..512f2c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,14 +4037,13 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
cmd)
goto cleanup;
}
- if (weight) {
- nparams++;
- if (weight 0) {
- vshError(ctl, _(Invalid value of %d for I/O weight), weight);
- goto cleanup;
- }
+ if (weight= 0) {
+ vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+ goto cleanup;
}
+ nparams++;
+
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL,nparams, flags) != 0) {
Hmm, the above patch will introduce new issue, if blkiotune without any
weight option,
vshError will also be hit, that is not we expected.
The reason of root is vshCommandOptInt will assign 0 to weight if option
not found
and not required, so we need to specifically deal with 0 value, I will
renew consider this
issue, so cancel this patch.
Actually, vshCommandOptInt (and all virsh opts parsing functions),
touches
given variable only iff found valid. In all other cases the variable
remains untouched. It's the initialization which set weight to 0.
The reason for such return values is, we can firstly initialize variable,
then call option parsing function over it which will change it iff
everything's fine:
int var = 10;
if (vshCommandOptInt(cmd, int-attr, var) 0) {
/* error, e.g. string value given */
goto cleanup;
}
/* here, var may be 10, iff int-attr was not given,
or other value if it was */
Or, if int-att is required, we just change the test:
if (vshCommandOptInt(cmd, int-attr, var) = 0) {
Hi Michal,
if option not found and not required (@value untouched), it means
weight value is
still 0 (keep initial 0 value), and if we only want to get weight value,
_(Unable to parse integer parameter)) will be also hit, this is not we
expected.
Thanks a lot,
Alex
Perhaps this will help :)
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: avoid missing zero value judgement in cmdBlkiotune
On 28.07.2011 11:04, Alex Jia wrote:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
I liked the first verion more than this. Although it is not shown in
this snippet,
nparams is initialized to zero
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..512f2c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,14 +4037,13 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
-vshError(ctl, _(Invalid value of %d for I/O weight), weight);
-goto cleanup;
-}
+if (weight= 0) {
+vshError(ctl, _(Invalid value of %d for I/O weight), weight);
+goto cleanup;
}
and therefore doing this
+nparams++;
+
will invalidate this test.
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL,nparams, flags) != 0) {
The better way of dealing with this issue IMO is to switch from
if (weight) {
to test if 'weight' parameter was given (vshCommandOptInt() returns 0)
and after this
check for impermissible values. And after the latter test (still inside
'weight argument
given') increment nparams:
if (weight_given) {
if (weight=0) {
report error; goto cleanup;
nparams++;
}
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] virsh: avoid missing zero value judgement in cmdBlkiotune
On 07/28/2011 05:50 PM, Wen Congyang wrote:
At 07/28/2011 05:52 PM, Alex Jia Write:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune, when
weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..f24050d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,12 +4037,12 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
goto cleanup;
}
-if (weight) {
-nparams++;
-if (weight 0) {
+if (vshCommandOptInt(cmd, weight,weight) 0) {
Why you call vshCommandOptInt(cmd, weight,weight) again?
Make sure weight indeed exists and then checking whether its value is right.
if do it like this:
if (weight= 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
nparams++;
..
when I run virsh blkiotune ${guestname} --weight 0, I will not hit this error,
and when I get weight value by virsh blkiotune ${guestname}, this error will be
hit, because vshCommandOptInt(cmd, weight,weight) will return 0 and unchange
weight value, it means weight will keep initial 0 value, hence vshError will be
raise.
Thanks,
Alex
+if (weight= 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
+nparams++;
}
if (nparams == 0) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] virsh: avoid missing zero value judgement in cmdBlkiotune
On 28.07.2011 12:38, Alex Jia wrote:
On 07/28/2011 05:50 PM, Wen Congyang wrote:
At 07/28/2011 05:52 PM, Alex Jia Write:
* tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune,
when
weight is equal to 0, the cmdBlkiotune will not raise any error
information
when judge weight value first time, and execute else branch to judge
weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field
is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters
function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..f24050d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,12 +4037,12 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
cmd)
goto cleanup;
}
- if (weight) {
- nparams++;
- if (weight 0) {
+ if (vshCommandOptInt(cmd, weight,weight) 0) {
Why you call vshCommandOptInt(cmd, weight,weight) again?
Make sure weight indeed exists and then checking whether its value is
right.
if do it like this:
if (weight= 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
nparams++;
..
when I run virsh blkiotune ${guestname} --weight 0, I will not hit this
error,
and when I get weight value by virsh blkiotune ${guestname}, this error
will be
hit, because vshCommandOptInt(cmd, weight,weight) will return 0 and
unchange
weight value, it means weight will keep initial 0 value, hence vshError
will be
raise.
Thanks,
Alex
Our point is, you can store the return value of vshCommandOptInt (a few
lines above)
and then just check against it instead of running the function again.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
* tools/virsh.c: fix missing zero value judgement in cmdBlkiotune and correct
vshError information.
when weight is equal to 0, the cmdBlkiotune will not raise any error
information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jia a...@redhat.com
---
tools/virsh.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..feb45de 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4004,6 +4004,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
virDomainPtr dom;
int weight = 0;
int nparams = 0;
+int rv = 0;
unsigned int i = 0;
virTypedParameterPtr params = NULL, temp = NULL;
bool ret = false;
@@ -4031,15 +4032,15 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshCommandOptInt(cmd, weight, weight) 0) {
+if ((rv = vshCommandOptInt(cmd, weight, weight)) 0) {
vshError(ctl, %s,
- _(Unable to parse integer parameter));
+ _(Unable to parse non-integer parameter));
goto cleanup;
}
-if (weight) {
+if (rv 0) {
nparams++;
-if (weight 0) {
+if (weight = 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
--- bootstrap.conf |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index f006a47..3b105b1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ configmake count-one-bits crypto/md5 dirname-lgpl +environ fclose fcntl-h ffs -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
On 28.07.2011 13:13, Alex Jia wrote:
* tools/virsh.c: fix missing zero value judgement in cmdBlkiotune and correct
vshError information.
when weight is equal to 0, the cmdBlkiotune will not raise any error
information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..feb45de 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4004,6 +4004,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
virDomainPtr dom;
int weight = 0;
int nparams = 0;
+int rv = 0;
unsigned int i = 0;
virTypedParameterPtr params = NULL, temp = NULL;
bool ret = false;
@@ -4031,15 +4032,15 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshCommandOptInt(cmd, weight,weight) 0) {
+if ((rv = vshCommandOptInt(cmd, weight,weight)) 0) {
vshError(ctl, %s,
- _(Unable to parse integer parameter));
+ _(Unable to parse non-integer parameter));
Why this change?
goto cleanup;
}
-if (weight) {
+if (rv 0) {
nparams++;
-if (weight 0) {
+if (weight= 0) {
vshError(ctl, _(Invalid value of %d for I/O weight), weight);
goto cleanup;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-0.9.1 to 0.9.3-r1: managedsave/save won't start/restore at saved state
The 27/07/11, Nicolas Sebrecht wrote: I'm seeing strange behaviour, here. Any guests saved using both managedsave and save commands from virsh won't restore at saved state. A new full boot sequence happen. - Tested against libvirt v0.9.1, v0.9.2, v0.9.3-r1 (Gentoo) - Confirmed on three different hosts Gentoo amd64 systems. - Tested with gentoo and ubuntu guests. - Nothing relevant in /var/log/libvirt/libvirt.log or /var/log/libvirt/qemu/dom.log The state file /var/lib/libvirt/qemu/save/dom.save exists and is deleted when 'virsh start' is called. The new boot sequence is confirmed by : - VNC console checks - previous screen sessions lost - uptime I've open a bug at https://bugs.gentoo.org/show_bug.cgi?id=376333 but had no answer. Any idea on what could happen or how to inspect it? I've found another Gentoo host system with same version of libvirt deployed where managedsave works. I'll investigate on my side to understand what differs between them. -- Nicolas Sebrecht -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h
2011/7/26 Eric Blake ebl...@redhat.com: On 07/26/2011 02:45 PM, Matthias Bolte wrote: +++ b/configure.ac @@ -2011,8 +2011,16 @@ dnl Enable building libvirtd? AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd = xyes]) dnl Check for gettext - don't go any newer than what RHEL 5 supports +dnl +dnl save and restore CPPFLAGS around gettext check as the internal iconv +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting +dnl in the build picking up previously installed libvirt/libvirt.h instead +dnl of the correct one from the soucre tree + +save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) +CPPFLAGS=$save_CPPFLAGS But I'm still worried that we have to use -I/usr/local/include somewhere in the command line, which means we would have to modify src/Makefile.am (and friends) to have: INCLUDES= ... $(GETTEXT_CPPFLAGS) where GETTEXT_CPPFLAGS is substituted with the difference in $save_CPPFLAGS and $CPPFLAGS prior to the point where we restore $CPPFLAGS. That's probably what we should do here. Now how to compute this difference? When we assume that save_CPPFLAGS and CPPFLAGS have a common prefix that we need to strip to get GETTEXT_CPPFLAGS then we could do it like this echo $(CPPFLAGS) | cut -c 1-`expr length $(save_CPPFLAGS)` --complement - This is probably neither the best nor a robustest way to do this. Do you have a better idea? Yep (although I haven't tested it thoroughly): save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) GETTEXT_CPPFLAGS= if test x$save_CPPFLAGS != x$CPPFLAGS; then set dummy $CPPFLAGS for var do case $var in $save_CPPFLAGS ) ;; *) GETTEXT_CPPFLAGS=$GETTEXT_CPPFLAGS $var ;; esac done fi CPPFLAGS=$save_CPPFLAGS Okay, this works for libvirt itself, but it fails for make check as the global CPPFLAGS also affect gnulib, but with this patch it doesn't contain the gettext related parts anymore and the gnulib tests fail to find libintl.h because of this. Making check in gnulib/tests Making check in . In file included from wait-process.c:37: ../../gnulib/lib/gettext.h:28:22: error: libintl.h: No such file or directory Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this? I attached a preliminary patch. -- Matthias Bolte http://photron.blogspot.com diff --git a/configure.ac b/configure.ac index ac34efc..b61c8e4 100644 --- a/configure.ac +++ b/configure.ac @@ -2055,8 +2055,30 @@ dnl Enable building libvirtd? AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd = xyes]) dnl Check for gettext - don't go any newer than what RHEL 5 supports +dnl +dnl save and restore CPPFLAGS around gettext check as the internal iconv +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting +dnl in the build picking up previously installed libvirt/libvirt.h instead +dnl of the correct one from the soucre tree. +dnl compute the difference between save_CPPFLAGS and CPPFLAGS and append it +dnl to INCLUDES in order to preserve changes made by gettext but in a place +dnl that does not break the build +save_CPPFLAGS=$CPPFLAGS AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) +GETTEXT_CPPFLAGS= +if test x$save_CPPFLAGS != x$CPPFLAGS; then + set dummy $CPPFLAGS; shift + for var + do + case $var in + $save_CPPFLAGS ) ;; + *) GETTEXT_CPPFLAGS=$GETTEXT_CPPFLAGS $var ;; + esac + done +fi +CPPFLAGS=$save_CPPFLAGS +AC_SUBST([GETTEXT_CPPFLAGS]) ALL_LINGUAS=`cd $srcdir/po /dev/null ls *.po | sed 's+\.po$++'` diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 63c731e..d81c29c 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,5 +1,15 @@ ## Process this file with automake to produce Makefile.in +INCLUDES = \ + -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ + -I$(top_srcdir)/include -I$(top_builddir)/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/src/util \ + -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/rpc \ + -I$(top_srcdir)/src/remote \ + $(GETTEXT_CPPFLAGS) + CLEANFILES = DAEMON_GENERATED = \ @@ -79,13 +89,6 @@ libvirtd_SOURCES = $(DAEMON_SOURCES) #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L libvirtd_CFLAGS = \ - -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ - -I$(top_srcdir)/include -I$(top_builddir)/include \ - -I$(top_srcdir)/src \ - -I$(top_srcdir)/src/util \ - -I$(top_srcdir)/src/conf \ - -I$(top_srcdir)/src/rpc \ - -I$(top_srcdir)/src/remote \ $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ $(XDR_CFLAGS) $(POLKIT_CFLAGS) \ $(WARN_CFLAGS) \ diff --git a/python/Makefile.am b/python/Makefile.am index 51f005d..063b1bf 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -6,7 +6,8 @@ INCLUDES = \ $(PYTHON_INCLUDES) \ -I$(top_srcdir)/include \ -I$(top_builddir)/include \ - -I$(top_builddir)/$(subdir) +
Re: [libvirt] PCI devices passthough to LXC containers using libvirt
Hi
Thanks for the reply.
I think the links that you provided show
how to deal with pci devices in case the hypervisor is kvm.
Please correct me if I am wrong.
But I am using LXC containers.I have skimmed through
the libvirt lxc driver code and found no functionality of
allowing specified devices into a container exists
other than currently where only hard coded devices are allowed
which can be seen in the file lxc_controller.c
struct cgroup_device_policy devices[] = {
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM},
{'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_TTY},
{'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX},
{0, 0, 0}};
Please confirm this or let me know if there is any other interface libvirt_lxc
provides to allow specific pci/other devices into container.
Thanks in advance
Regards
Devendra
- Original Message -
From: Alex Jia a...@redhat.com
To: Devendra K. Modium dmod...@isi.edu
Sent: Thursday, July 28, 2011 3:22:36 AM
Subject: Re: [libvirt] PCI devices passthough to LXC containers using libvirt
On 07/28/2011 05:13 AM, Devendra K. Modium wrote:
Hi All
Please let me know if anyone have given access to
PCI devices for a LXC container.
I have tried getting the xml from
virsh nodedev-dumpxml pci_device and
added to the libvirt xml file as shown below
device
namepci__03_00_0/name
parentpci__00_03_0/parent
driver
namenvidia/name
/driver
capability type='pci'
domain0/domain
bus3/bus
slot0/slot
function0/function
product id='0x06fd' /
vendor id='0x10de'nVidia Corporation/vendor
/capability
/device
You shouldn't directly add the above xml to guest xml configuration, here is a
usage,
it should be helpful for you:
http://libvirt.org/formatdomain.html#elementsUSB
http://fedoraproject.org/wiki/Category:Virtualization_KVM_PCI_Device_Assignment_Test_Cases
Good Luck!
Alex
But it didn't work. I see the logs and it says
couldn't get physical and virtual functions of these devices with error
get_physical_function_linux:323 : Attempting to get SR IOV physical function
for device with sysfs path '/sys/devices/pci:00/:00:00.0'
16:48:34.033: 13802: debug : get_sriov_function:270 : Attempting to resolve
device path from device link '/sys/devices/pci:00/:00:00.0/physfn'
16:48:34.033: 13802: debug : get_sriov_function:274 : SR IOV function link
'/sys/devices/pci:00/:00:00.0/physfn' does not exist
16:48:34.033: 13802: debug : get_virtual_functions_linux:348 : Attempting to
get SR IOV virtual functions for devicewith sysfs path
'/sys/devices/pci:00/:00:00.0'
If anyone got some guidelines how to debug, please let me know.
Thanks in advance
Regards
Devendra
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC] virsh: Add option to undefine storage with domains
Adds an option to virsh undefine command to undefine managed
storage volumes along with (inactive) domains. Storage volumes
are enumerated and the user may interactivly choose volumes
to delete.
Unmanaged volumes are listed and the user shall delete them
manualy.
---
I marked this as a RFC because I am concerned about my naming scheme of the
added parameters.
I couldn't decide which of the following volumes/storage/disks/... to use.
I'd appreciate your
comments on this.
This is my second approach to this problem after I got some really good
critique from Eric,
Daniel and Dave. The user has the choice to activate an interactive mode, that
allows to select on a
per-device basis which volumes/disks to remove along with the domain.
To avoid possible problems, I only allowed to remove storage for inactive
domains and unmanaged
images (which sidetracked me a lot on my previous attempt) are left to a action
of the user.
(the user is notified about any unmanaged image for the domain).
My next concern is about interactive of the user. I tried to implement a
boolean query function,
but I'd like to know if I took the right path, as I couldn't find any example
in virsh from which I
could learn.
Thanks for your comments (and time) :)
Peter Krempa
tools/virsh.c | 265 +++--
1 files changed, 259 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 61f69f0..3795d2b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -295,6 +295,9 @@ static int vshCommandOptULongLong(const vshCmd *cmd, const
char *name,
static bool vshCommandOptBool(const vshCmd *cmd, const char *name);
static const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd,
const vshCmdOpt *opt);
+static int vshInteractiveBoolPrompt(vshControl *ctl,
+const char *prompt,
+ bool *confirm);
#define VSH_BYID (1 1)
#define VSH_BYUUID (1 2)
@@ -1422,6 +1425,8 @@ static const vshCmdInfo info_undefine[] = {
static const vshCmdOptDef opts_undefine[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name or uuid)},
{managed-save, VSH_OT_BOOL, 0, N_(remove domain managed state file)},
+{disks, VSH_OT_BOOL, 0, N_(remove associated disk images managed in
storage pools (interactive))},
+{disks-all, VSH_OT_BOOL, 0, N_(remove all associated disk images
managed in storage pools)},
{NULL, 0, 0, NULL}
};
@@ -1434,9 +1439,25 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
int id;
int flags = 0;
int managed_save = vshCommandOptBool(cmd, managed-save);
+int remove_disks = vshCommandOptBool(cmd, disks);
+int remove_all_disks = vshCommandOptBool(cmd, disks-all);
int has_managed_save = 0;
int rc = -1;
+char *domxml;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlXPathObjectPtr obj = NULL;
+xmlNodePtr cur = NULL;
+int i = 0;
+char *source = NULL;
+char *target = NULL;
+char *type = NULL;
+xmlBufferPtr xml_buf = NULL;
+virStorageVolPtr volume = NULL;
+int state;
+bool confirm = false;
+
if (managed_save)
flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
@@ -1475,15 +1496,172 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
}
}
-if (flags == -1) {
-if (has_managed_save == 1) {
+
+if (flags == -1 has_managed_save == 1) {
+vshError(ctl,
+ _(Refusing to undefine while domain managed save
+ image exists));
+virDomainFree(dom);
+return false;
+}
+
+if (remove_disks || remove_all_disks) {
+if ((state = vshDomainState(ctl, dom, NULL)) 0) {
+vshError(ctl, _(Failed to get domain state));
+goto disk_error;
+}
+
+/* removal of storage is possible only for inactive domains */
+if (!((state == VIR_DOMAIN_SHUTOFF) ||
+ (state == VIR_DOMAIN_CRASHED))) {
vshError(ctl,
- _(Refusing to undefine while domain managed save
- image exists));
-virDomainFree(dom);
-return false;
+ _(Domain needs to be inactive to delete it with
associated storage));
+goto disk_error;
+}
+
+if (remove_disks !ctl-imode) {
+vshError(ctl, %s\n, _(Option --disks is available only in
interactive mode));
+goto disk_error;
+}
+
+domxml = virDomainGetXMLDesc(dom, 0);
+if (!domxml) {
+vshError(ctl, %s, _(Failed to get disk information));
+goto disk_error;
+}
+
+xml = xmlReadDoc((const xmlChar *) domxml, domain.xml, NULL,
+ XML_PARSE_NOENT |
+ XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+VIR_FREE(domxml);
+
+if
Re: [libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
On 07/28/2011 05:08 AM, Matthias Bolte wrote: --- bootstrap.conf |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index f006a47..3b105b1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ configmake count-one-bits crypto/md5 dirname-lgpl +environ ACK. POSIX is clear that 'environ' is not declared by any standard header unless you use vendor extensions, whereas the gnulib module 'environ' guarantees the declaration in unistd.h to match the glibc extension when _GNU_SOURCE is defined (that is, the gnulib module is the vendor extension that we need to get the declaration visible on BSD). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h
On 07/28/2011 06:55 AM, Matthias Bolte wrote: Okay, this works for libvirt itself, but it fails for make check as the global CPPFLAGS also affect gnulib, but with this patch it doesn't contain the gettext related parts anymore and the gnulib tests fail to find libintl.h because of this. Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this? Fortunately, I know how to fix that - it involves changing our gnulib-tool to spit out gnulib.mk to be included from a wrapper Makefile.am that we manage, rather than our current practice of letting gnulib-tool spit out the complete Makefile.am (coreutils.git does the same thing). I attached a preliminary patch. I'll submit a v2 based on your patch shortly. +dnl save and restore CPPFLAGS around gettext check as the internal iconv +dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting +dnl in the build picking up previously installed libvirt/libvirt.h instead +dnl of the correct one from the soucre tree. including this typo fix: s/soucre/source/ Everything else you had looks good, just missing the gnulib handling. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
2011/7/28 Eric Blake ebl...@redhat.com: On 07/28/2011 05:08 AM, Matthias Bolte wrote: --- bootstrap.conf | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index f006a47..3b105b1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ configmake count-one-bits crypto/md5 dirname-lgpl +environ ACK. POSIX is clear that 'environ' is not declared by any standard header unless you use vendor extensions, whereas the gnulib module 'environ' guarantees the declaration in unistd.h to match the glibc extension when _GNU_SOURCE is defined (that is, the gnulib module is the vendor extension that we need to get the declaration visible on BSD). At least all tests compile on FreeBSD again. But most of the SSH cases in virnetmessagetest are failing and I don't understand why yet. Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Add gnulib environ module for the commandtest
On 07/28/2011 07:52 AM, Matthias Bolte wrote: At least all tests compile on FreeBSD again. But most of the SSH cases in virnetmessagetest are failing and I don't understand why yet. Could it be a PATH vs. exec() issue, where BSD ends up doing a slightly different PATH search and not executing the dummy 'ssh' script from our test directory? Does a ktrace (or truss or strace or however it's spelled) shed any light? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PCI devices passthough to LXC containers using libvirt
Quoting Devendra K. Modium (dmod...@isi.edu):
Hi
Thanks for the reply.
I think the links that you provided show
how to deal with pci devices in case the hypervisor is kvm.
Please correct me if I am wrong.
But I am using LXC containers.I have skimmed through
the libvirt lxc driver code and found no functionality of
allowing specified devices into a container exists
other than currently where only hard coded devices are allowed
which can be seen in the file lxc_controller.c
struct cgroup_device_policy devices[] = {
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM},
{'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM},
{'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_TTY},
{'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX},
{0, 0, 0}};
Please confirm this or let me know if there is any other interface
libvirt_lxc
provides to allow specific pci/other devices into container.
The qemu driver allows for this list to be specified in the
config file. You could send a patch for the lxc driver to do
the same. You also could go further and add xml format to
add extra entries.
But as this is implemented using cgroups, the other thing you
can do is to manually, after you start the container, add the
devices whitelist entries yourself. Depending on how you're
using it this might suffice...
-serge
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix memory leak on metadata fetching
On 28.07.2011 16:00, Eric Blake wrote: On 07/28/2011 07:56 AM, Michal Privoznik wrote: As written in virStorageFileGetMetadataFromFD decription, caller must free metadata after use. Qemu driver miss this and therefore leak metadata which can grow to huge mem leak if somebody query for blockInfo a lot. --- src/qemu/qemu_driver.c | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) ACK. Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC] virsh: Add option to undefine storage with domains
On Thu, Jul 28, 2011 at 03:41:01PM +0200, Peter Krempa wrote:
Adds an option to virsh undefine command to undefine managed
storage volumes along with (inactive) domains. Storage volumes
are enumerated and the user may interactivly choose volumes
to delete.
Unmanaged volumes are listed and the user shall delete them
manualy.
---
I marked this as a RFC because I am concerned about my naming scheme of
the added parameters.
I couldn't decide which of the following volumes/storage/disks/... to use.
I'd appreciate your
comments on this.
This is my second approach to this problem after I got some really good
critique from Eric,
Daniel and Dave. The user has the choice to activate an interactive mode,
that allows to select on a
per-device basis which volumes/disks to remove along with the domain.
To avoid possible problems, I only allowed to remove storage for inactive
domains and unmanaged
I think you mean managed images, right?
images (which sidetracked me a lot on my previous attempt) are left to a
action of the user.
(the user is notified about any unmanaged image for the domain).
My next concern is about interactive of the user. I tried to implement a
boolean query function,
but I'd like to know if I took the right path, as I couldn't find any example
in virsh from which I
could learn.
We haven't previously implemented that much interactivity in virsh,
and I'm not sure I want to go down that road. virsh is generally a
pretty straight passthrough to the API. I'm sure others will have an
opinion on that question as well.
Thanks for your comments (and time) :)
A few comments inline.
Dave
Peter Krempa
tools/virsh.c | 265
+++--
1 files changed, 259 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 61f69f0..3795d2b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -295,6 +295,9 @@ static int vshCommandOptULongLong(const vshCmd *cmd,
const char *name,
static bool vshCommandOptBool(const vshCmd *cmd, const char *name);
static const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd,
const vshCmdOpt *opt);
+static int vshInteractiveBoolPrompt(vshControl *ctl,
+const char *prompt,
+ bool *confirm);
#define VSH_BYID (1 1)
#define VSH_BYUUID (1 2)
@@ -1422,6 +1425,8 @@ static const vshCmdInfo info_undefine[] = {
static const vshCmdOptDef opts_undefine[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name or uuid)},
{managed-save, VSH_OT_BOOL, 0, N_(remove domain managed state file)},
+{disks, VSH_OT_BOOL, 0, N_(remove associated disk images managed in
storage pools (interactive))},
+{disks-all, VSH_OT_BOOL, 0, N_(remove all associated disk images
managed in storage pools)},
I think it would be clearer stated as remove all associated storage
volumes, and correspondingly, call the option storage-volumes.
{NULL, 0, 0, NULL}
};
@@ -1434,9 +1439,25 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
int id;
int flags = 0;
int managed_save = vshCommandOptBool(cmd, managed-save);
+int remove_disks = vshCommandOptBool(cmd, disks);
+int remove_all_disks = vshCommandOptBool(cmd, disks-all);
int has_managed_save = 0;
int rc = -1;
+char *domxml;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+xmlXPathObjectPtr obj = NULL;
+xmlNodePtr cur = NULL;
+int i = 0;
+char *source = NULL;
+char *target = NULL;
+char *type = NULL;
+xmlBufferPtr xml_buf = NULL;
+virStorageVolPtr volume = NULL;
+int state;
+bool confirm = false;
+
if (managed_save)
flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
@@ -1475,15 +1496,172 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
}
}
-if (flags == -1) {
-if (has_managed_save == 1) {
+
+if (flags == -1 has_managed_save == 1) {
+vshError(ctl,
+ _(Refusing to undefine while domain managed save
+ image exists));
How does this interact with --managed-save? Can a user specify both
--managed-save and --disks to remove both managed save and storage volumes?
+virDomainFree(dom);
+return false;
+}
+
+if (remove_disks || remove_all_disks) {
+if ((state = vshDomainState(ctl, dom, NULL)) 0) {
+vshError(ctl, _(Failed to get domain state));
+goto disk_error;
+}
+
+/* removal of storage is possible only for inactive domains */
+if (!((state == VIR_DOMAIN_SHUTOFF) ||
+ (state == VIR_DOMAIN_CRASHED))) {
vshError(ctl,
- _(Refusing to undefine while domain managed save
- image exists));
-
Re: [libvirt] [PATCH] build: avoid type-punning compiler warning
On 07/28/2011 04:11 AM, Matthias Bolte wrote: 2011/7/27 Eric Blakeebl...@redhat.com: On RHEL 5, with gcc 4.1.2: rpc/virnetsaslcontext.c: In function 'virNetSASLSessionUpdateBufSize': rpc/virnetsaslcontext.c:396: warning: dereferncing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] * src/rpc/virnetsaslcontext.c (virNetSASLSessionUpdateBufSize): Use a union to work around gcc warning. --- src/rpc/virnetsaslcontext.c | 11 +++ 1 files changed, 7 insertions(+), 4 deletions(-) ACK. Thanks; pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix nested job with driver lock held
On 07/27/2011 09:58 PM, Laine Stump wrote: On 07/27/2011 10:04 PM, Eric Blake wrote: qemuMigrationUpdateJobStatus (called in a loop by migration and save tasks) uses qemuDomainObjEnterMonitorWithDriver; however, that function ended up starting a nested job without releasing the driver. Since no one else is making nested calls, we can inline the internal functions to properly track driver_locked. ACK. Thanks; pushed. Now to test Wen's patch to hopefully solve the managedsave crash. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
On 07/28/2011 07:20 PM, Michal Privoznik wrote:
On 28.07.2011 13:13, Alex Jia wrote:
* tools/virsh.c: fix missing zero value judgement in cmdBlkiotune and
correct
vshError information.
when weight is equal to 0, the cmdBlkiotune will not raise any
error information
when judge weight value first time, and execute else branch to
judge weight
value again, strncpy(temp-field, VIR_DOMAIN_BLKIO_WEIGHT,
sizeof(temp-field))
will be not executed for ever. However, if and only if
param-field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying
qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jiaa...@redhat.com
---
tools/virsh.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..feb45de 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4004,6 +4004,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
virDomainPtr dom;
int weight = 0;
int nparams = 0;
+int rv = 0;
unsigned int i = 0;
virTypedParameterPtr params = NULL, temp = NULL;
bool ret = false;
@@ -4031,15 +4032,15 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshCommandOptInt(cmd, weight,weight) 0) {
+if ((rv = vshCommandOptInt(cmd, weight,weight)) 0) {
vshError(ctl, %s,
- _(Unable to parse integer parameter));
+ _(Unable to parse non-integer parameter));
Why this change?
when I give a non-integer to weight value such as
virsh blkiotune $domain --weight demo, vshError will be hit and raise
Unable to parse integer parameter, 'demo' is a string not integer, it
should
be only can parse integer parameter, so I think it should be changed
like above,
please correct me again if I'm wrong again.
BTW, except this, are others okay?
Thanks,
Alex
goto cleanup;
}
-if (weight) {
+if (rv 0) {
nparams++;
-if (weight 0) {
+if (weight= 0) {
vshError(ctl, _(Invalid value of %d for I/O weight),
weight);
goto cleanup;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
On 07/28/2011 08:25 AM, ajia wrote: - _(Unable to parse integer parameter)); + _(Unable to parse non-integer parameter)); Why this change? when I give a non-integer to weight value such as virsh blkiotune $domain --weight demo, vshError will be hit and raise Unable to parse integer parameter, 'demo' is a string not integer, it should be only can parse integer parameter, so I think it should be changed like above, please correct me again if I'm wrong again. If you wanted to do that, then we need something longer: Unable to parse, non-integer encountered where integer parameter expected But I think the shorter version is accurate - our intent is to tell the user this option requires an integer parameter, but the input you gave me could not be parsed as an integer. So I don't think the error message should change. BTW, except this, are others okay? Yes, so I pushed with that one line fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
On 07/28/2011 10:31 PM, Eric Blake wrote: On 07/28/2011 08:25 AM, ajia wrote: - _(Unable to parse integer parameter)); + _(Unable to parse non-integer parameter)); Why this change? when I give a non-integer to weight value such as virsh blkiotune $domain --weight demo, vshError will be hit and raise Unable to parse integer parameter, 'demo' is a string not integer, it should be only can parse integer parameter, so I think it should be changed like above, please correct me again if I'm wrong again. If you wanted to do that, then we need something longer: Unable to parse, non-integer encountered where integer parameter expected But I think the shorter version is accurate - our intent is to tell the user this option requires an integer parameter, but the input you gave me could not be parsed as an integer. So I don't think the error message should change. BTW, except this, are others okay? Yes, so I pushed with that one line fixed. Eric, thanks :) Regards, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] freebsd: Avoid /bin/true in commandtest
Rely on PATH and use just true, because on FreeBSD it's /usr/bin/true.
---
tests/commanddata/test16.log |2 +-
tests/commandtest.c |6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/commanddata/test16.log b/tests/commanddata/test16.log
index 2433a4d..7088165 100644
--- a/tests/commanddata/test16.log
+++ b/tests/commanddata/test16.log
@@ -1 +1 @@
-A=B /bin/true C
+A=B true C
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 2e800ae..9ba53b8 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -566,9 +566,9 @@ cleanup:
*/
static int test16(const void *unused ATTRIBUTE_UNUSED)
{
-virCommandPtr cmd = virCommandNew(/bin/true);
+virCommandPtr cmd = virCommandNew(true);
char *outactual = NULL;
-const char *outexpect = A=B /bin/true C;
+const char *outexpect = A=B true C;
int ret = -1;
int fd = -1;
@@ -610,7 +610,7 @@ cleanup:
*/
static int test17(const void *unused ATTRIBUTE_UNUSED)
{
-virCommandPtr cmd = virCommandNew(/bin/true);
+virCommandPtr cmd = virCommandNew(true);
int ret = -1;
char *outbuf;
char *errbuf;
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Unify style of test skipping code
Prefer 'return EXIT_AM_SKIP' over 'exit(EXIT_AM_SKIP)'.
Prefer 'int main(void)' over 'int main(int argc, char **argv)'.
Fix mymain signature in commandtest and nodeinfotest.
---
tests/commandtest.c | 10 +-
tests/nodeinfotest.c | 10 +-
tests/qemuargv2xmltest.c |6 +-
tests/qemuxml2xmltest.c |6 +-
tests/virnettlscontexttest.c |6 --
tests/virshtest.c| 26 +-
6 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index ef2850d..2e800ae 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -38,10 +38,10 @@
#ifdef WIN32
-static int
-mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+int
+main(void)
{
-exit (EXIT_AM_SKIP);
+return EXIT_AM_SKIP;
}
#else
@@ -814,6 +814,6 @@ mymain(void)
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
-#endif /* !WIN32 */
-
VIRT_TEST_MAIN(mymain)
+
+#endif /* !WIN32 */
diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index 5abee92..448e072 100644
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -15,10 +15,10 @@
defined(__amd64__) || \
defined(__i386__)))
-static int
-mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+int
+main(void)
{
-exit (EXIT_AM_SKIP);
+return EXIT_AM_SKIP;
}
#else
@@ -130,6 +130,6 @@ mymain(void)
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
-#endif /* __linux__ */
-
VIRT_TEST_MAIN(mymain)
+
+#endif /* __linux__ */
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index bade95d..c2b6cf2 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -239,6 +239,10 @@ VIRT_TEST_MAIN(mymain)
#else
-int main (void) { return (EXIT_AM_SKIP); }
+int
+main(void)
+{
+return EXIT_AM_SKIP;
+}
#endif /* WITH_QEMU */
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index f1900c5..461fd0d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -204,6 +204,10 @@ VIRT_TEST_MAIN(mymain)
#else
-int main (void) { exit (EXIT_AM_SKIP); }
+int
+main(void)
+{
+return EXIT_AM_SKIP;
+}
#endif /* WITH_QEMU */
diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c
index 12ecf1e..520b006 100644
--- a/tests/virnettlscontexttest.c
+++ b/tests/virnettlscontexttest.c
@@ -1249,9 +1249,11 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
+
int
-main(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+main(void)
{
-exit (EXIT_AM_SKIP);
+return EXIT_AM_SKIP;
}
+
#endif
diff --git a/tests/virshtest.c b/tests/virshtest.c
index e22e582..de5138c 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -9,7 +9,17 @@
#include util.h
#include testutils.h
-#define DOM_UUID ef861801-45b9-11cb-88e3-afbfe5370493
+#ifdef WIN32
+
+int
+main(void)
+{
+return EXIT_AM_SKIP;
+}
+
+#else
+
+# define DOM_UUID ef861801-45b9-11cb-88e3-afbfe5370493
static const char *dominfo_fc4 = \
Id: 2\n\
@@ -71,13 +81,13 @@ cleanup:
return result;
}
-#define VIRSH_DEFAULT ../tools/virsh, \
+# define VIRSH_DEFAULT ../tools/virsh, \
--connect, \
test:///default
static char *custom_uri;
-#define VIRSH_CUSTOM ../tools/virsh, \
+# define VIRSH_CUSTOM ../tools/virsh, \
--connect, \
custom_uri
@@ -224,10 +234,6 @@ mymain(void)
{
int ret = 0;
-#ifdef WIN32
-exit (EXIT_AM_SKIP);
-#endif
-
if (virAsprintf(custom_uri, test://%s/../examples/xml/test/testnode.xml,
abs_srcdir) 0)
return EXIT_FAILURE;
@@ -298,7 +304,7 @@ mymain(void)
/* It's a bit awkward listing result before argument, but that's a
* limitation of C99 vararg macros. */
-#define DO_TEST(i, result, ...) \
+# define DO_TEST(i, result, ...) \
do {\
const char *myargv[] = { VIRSH_DEFAULT, __VA_ARGS__, NULL };\
const struct testInfo info = { myargv, result };\
@@ -380,10 +386,12 @@ mymain(void)
DO_TEST(30, --shell a\n,
echo \t '-'\-\ \t --shell \t a);
-#undef DO_TEST
+# undef DO_TEST
free(custom_uri);
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
VIRT_TEST_MAIN(mymain)
+
+#endif /* WIN32 */
--
1.7.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ANNOUNCE: virt-manager 0.9.0 and virtinst 0.600.0 released
I'm happy to announce two new releases: virt-manager 0.9.0: virt-manager is a desktop application for managing KVM and Xen virtual machines via libvirt. virtinst 0.600.0: virtinst is a collection of command line tools for provisioning libvirt virtual machines, including virt-install and virt-clone. The releases can be downloaded from: http://virt-manager.org/download.html The direct download links are: http://virt-manager.org/download/sources/virt-manager/virt-manager-0.9.0.tar.gz http://virt-manager.org/download/sources/virtinst/virtinst-0.600.0.tar.gz The virt-manager release includes: - Use a hiding toolbar for fullscreen mode - Use libguestfs to show guest packagelist and more (Richard W.M. Jones) - Basic 'New VM' wizard support for LXC guests - Remote serial console access (with latest libvirt) - Remote URL guest installs (with latest libvirt) - Add Hardware: Support filesystem devices - Add Hardware: Support smartcard devices (Marc-André Lureau) - Enable direct interface selection for qemu/kvm (Gerhard Stenzel) - Allow viewing and changing disk serial number The virtinst release includes: - virt-install: Various improvements to enable LXC/container guests: - New --filesystem option for filesystem devices - New --init option for container init path - New --container option (similar to --paravirt or --hvm) - virt-install: Make --location remotely (with latest libvirt) - virt-install: New --smartcard option for smartcard devices (Marc-André Lureau) - virt-install: New --numatune option for building guest numatune XML - virt-install: option to set --disk error_policy= - virt-install: option to set --disk serial= Thanks to everyone who has contributed to this release through testing, bug reporting, submitting patches, and otherwise sending in feedback! Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/2] conf: add listen subelement to domain graphics element
On 07/28/2011 12:11 AM, Laine Stump wrote: Once it's plugged in, thelisten element will be an optional replacement for the listen attribute that graphics elements already have. If thelisten element is type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', thelisten element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from. I think we're there. Thanks for putting up with the review churn. +++ b/docs/formatdomain.html.in @@ -2046,6 +2046,12 @@ qemu-kvm -net nic,model=? /dev/null lt;graphics type='vnc' port='5904'/gt; It looks fishy to have one... lt;graphics type='rdp' autoport='yes' multiUser='yes' /gt; lt;graphics type='desktop' fullscreen='yes'/gt; +lt;graphics type='vnc'gt; +lt;listen type='address' address='1.2.3.4'/gt; +lt;/graphicsgt; ...and then a second type='vnc' description (do we support multiple vnc graphics adapters on any existing hypervisor?). How about consolidating this part of the example into just: lt;graphics type='vnc' port='5904'gt; lt;listen type='address' address='1.2.3.4'/gt; lt;/graphicsgt; +dtcodeaddress/code/dt +ddifcodetype='address'/code, thecodeaddress/code +attribute will contain either an IP address or hostname (which +will be resolved to an IP address via a DNS query) to listen +on. In the live XML of a running domain, this attribute be s/be/will be/ ACK with those nits fixed, no v5 needed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Unify style of test skipping code
On 07/28/2011 09:52 AM, Matthias Bolte wrote: Prefer 'return EXIT_AM_SKIP' over 'exit(EXIT_AM_SKIP)'. Prefer 'int main(void)' over 'int main(int argc, char **argv)'. Fix mymain signature in commandtest and nodeinfotest. --- tests/commandtest.c | 10 +- tests/nodeinfotest.c | 10 +- tests/qemuargv2xmltest.c |6 +- tests/qemuxml2xmltest.c |6 +- tests/virnettlscontexttest.c |6 -- tests/virshtest.c| 26 +- 6 files changed, 41 insertions(+), 23 deletions(-) ACK, all cosmetic, so no problem pushing prior to 0.9.4. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] freebsd: Avoid /bin/true in commandtest
On 07/28/2011 09:52 AM, Matthias Bolte wrote: Rely on PATH and use just true, because on FreeBSD it's /usr/bin/true. What fun. The autoconf manual has this gem under true, apropos to the current patch: | when asked whether false is more portable than true Alexandre Oliva answered: | | In a sense, yes, because if it doesn't exist, the shell will produce an exit status of failure, which is correct for false, but not for true. http://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins --- tests/commanddata/test16.log |2 +- tests/commandtest.c |6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 2/2] qemu: support type=network in domain graphics listen
On 07/28/2011 12:11 AM, Laine Stump wrote: The domain XML now understands thelisten subelement of its graphics element (including when listen type='network'), and the network driver has an internal API that will turn a network name into an IP address, so the final logical step is to put the glue into the qemu driver so that when it is starting up a domain, if it finds listen type='network' network='xyz'/ in the XML, it will call the network driver to get an IPv4 address associated with network xyz, and tell qemu to listen for vnc (or spice) on that address rather than the default address (localhost). ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/2] conf: add listen subelement to domain graphics element
On 07/28/2011 01:09 PM, Eric Blake wrote: On 07/28/2011 12:11 AM, Laine Stump wrote: Once it's plugged in, thelisten element will be an optional replacement for the listen attribute that graphics elements already have. If thelisten element is type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', thelisten element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from. I think we're there. Thanks for putting up with the review churn. +++ b/docs/formatdomain.html.in @@ -2046,6 +2046,12 @@ qemu-kvm -net nic,model=? /dev/null lt;graphics type='vnc' port='5904'/gt; It looks fishy to have one... lt;graphics type='rdp' autoport='yes' multiUser='yes' /gt; lt;graphics type='desktop' fullscreen='yes'/gt; +lt;graphics type='vnc'gt; +lt;listen type='address' address='1.2.3.4'/gt; +lt;/graphicsgt; ...and then a second type='vnc' description (do we support multiple vnc graphics adapters on any existing hypervisor?). How about consolidating this part of the example into just: lt;graphics type='vnc' port='5904'gt; lt;listen type='address' address='1.2.3.4'/gt; lt;/graphicsgt; Good point. When I added the examples I was only thinking about putting in things that would demonstrate the syntax; I didn't think to consider if it was actually a practical setup. +dtcodeaddress/code/dt +ddifcodetype='address'/code, thecodeaddress/code +attribute will contain either an IP address or hostname (which +will be resolved to an IP address via a DNS query) to listen +on. In the live XML of a running domain, this attribute be s/be/will be/ ACK with those nits fixed, no v5 needed. Pushed (!!) Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 2/2] qemu: support type=network in domain graphics listen
On 07/28/2011 01:21 PM, Eric Blake wrote: On 07/28/2011 12:11 AM, Laine Stump wrote: The domain XML now understands thelisten subelement of its graphics element (including when listen type='network'), and the network driver has an internal API that will turn a network name into an IP address, so the final logical step is to put the glue into the qemu driver so that when it is starting up a domain, if it finds listen type='network' network='xyz'/ in the XML, it will call the network driver to get an IPv4 address associated with network xyz, and tell qemu to listen for vnc (or spice) on that address rather than the default address (localhost). ACK. Also Pushed. Thanks for all the review. Once again the final product is much better than the initial submission. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/3] save: support qemu modifying xml on domain save/restore
On 07/22/2011 12:21 AM, Eric Blake wrote:
With this, it is possible to update the path to a disk backing
image on either the save or restore action, without having to
binary edit the XML embedded in the state file.
This also modifies virDomainSave to output a smaller xml (only
the inactive xml, which is all the more virDomainRestore parses),
while still guaranteeing padding for most typical abi-compatible
xml replacements, necessary so that the next patch for
virDomainSaveImageDefineXML will not cause unnecessary
modifications to the save image file.
* src/qemu/qemu_driver.c (qemuDomainSaveInternal): Add parameter,
only use inactive state, and guarantee padding.
(qemuDomainSaveImageOpen): Add parameter.
(qemuDomainSaveFlags, qemuDomainManagedSave)
(qemuDomainRestoreFlags, qemuDomainObjRestore): Update callers.
---
v3: change virDomainSave to always output minimal information,
but with fixed padding added, so that save file modification
will always be more likely to succeed,
always be more likely. Heh.
Looking at this problem from the outside, it seems that if we wanted a
100% reliable solution, we would need to introduce the idea of a linked
header, which can be continued at the end of the file (of course that
wouldn't work if there are ever cases where the file is being read from
a pipe, and we can't seek, and it's entirely possible that 1024 is
always enough extra to ensure everything works).
and not generate quite
as many xml differences on round trips.
src/qemu/qemu_driver.c | 109 +--
1 files changed, 67 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1401967..2b1df6c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2193,7 +2193,7 @@ qemuCompressProgramName(int compress)
static int
qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
virDomainObjPtr vm, const char *path,
- int compressed, bool bypass_cache)
+ int compressed, bool bypass_cache, const char *xmlin)
{
char *xml = NULL;
struct qemud_save_header header;
@@ -2204,7 +2204,9 @@ qemuDomainSaveInternal(struct qemud_driver *driver,
virDomainPtr dom,
qemuDomainObjPrivatePtr priv;
struct stat sb;
bool is_reg = false;
+size_t len;
unsigned long long offset;
+unsigned long long pad;
int fd = -1;
uid_t uid = getuid();
gid_t gid = getgid();
@@ -2239,15 +2241,54 @@ qemuDomainSaveInternal(struct qemud_driver *driver,
virDomainPtr dom,
}
}
-/* Get XML for the domain */
-xml = virDomainDefFormat(vm-def, VIR_DOMAIN_XML_SECURE);
+/* Get XML for the domain. Restore needs only the inactive xml,
+ * including secure. We should get the same result whether xmlin
+ * is NULL or whether it was the live xml of the domain moments
+ * before. */
+if (xmlin) {
+virDomainDefPtr def = NULL;
+
+if (!(def = virDomainDefParseString(driver-caps, xmlin,
+QEMU_EXPECTED_VIRT_TYPES,
+VIR_DOMAIN_XML_INACTIVE))) {
+goto endjob;
+}
+if (!virDomainDefCheckABIStability(vm-def, def)) {
+virDomainDefFree(def);
+goto endjob;
+}
+xml = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE);
+} else {
+xml = virDomainDefFormat(vm-def, (VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_SECURE));
+}
if (!xml) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
%s, _(failed to get domain xml));
goto endjob;
}
-header.xml_len = strlen(xml) + 1;
+len = strlen(xml) + 1;
+offset = sizeof(header) + len;
+
+/* Due to way we append QEMU state on our header with dd,
+ * we need to ensure there's a 512 byte boundary. Unfortunately
+ * we don't have an explicit offset in the header, so we fake
+ * it by padding the XML string with NUL bytes. Additionally,
+ * we want to ensure that virDomainSaveImageDefineXML can supply
+ * slightly larger XML, so we add a miminum padding prior to
+ * rounding out to page boundaries.
+ */
+pad = 1024;
+pad += (QEMU_MONITOR_MIGRATE_TO_FILE_BS -
+((offset + pad) % QEMU_MONITOR_MIGRATE_TO_FILE_BS));
+if (VIR_EXPAND_N(xml, len, pad) 0) {
+virReportOOMError();
+goto endjob;
+}
+offset += pad;
+header.xml_len = len;
+/* Obtain the file handle. */
/* path might be a pre-existing block dev, in which case
* we need to skip the create step, and also avoid unlink
* in the failure case */
@@ -2268,29 +2309,6 @@ qemuDomainSaveInternal(struct qemud_driver *driver,
virDomainPtr dom,
}
}
-offset = sizeof(header) + header.xml_len;
-
-/* Due
Re: [libvirt] virNetClientPtr leak in remote driver
2011/7/27 Matthias Bolte matthias.bo...@googlemail.com:
doRemoteClose doesn't free the virNetClientPtr and this creates a
260kb leak per remote connection. This happens because
virNetClientFree doesn't remove the last ref, because virNetClientNew
creates the virNetClientPtr with a refcount of 2.
static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
const char *hostname)
{
[...]
client-refs = 1;
[...]
/* Set up a callback to listen on the socket data */
client-refs++;
if (virNetSocketAddIOCallback(client-sock,
VIR_EVENT_HANDLE_READABLE,
virNetClientIncomingEvent,
client,
virNetClientEventFree) 0) {
client-refs--;
VIR_DEBUG(Failed to add event watch, disabling events);
}
[...]
}
virNetClientNew adds a ref before calling virNetSocketAddIOCallback
but only removes it when virNetSocketAddIOCallback fails. This seems
wrong too me and the ref should be removed in the success case too.
The same pattern was added in 0302391ee643ad91fdc6d2ecf7e4cf0fc9724516
(Fix race in ref counting when handling RPC jobs)
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -763,10 +763,12 @@ readmore:
/* Send off to for normal dispatch to workers */
if (msg) {
+ client-refs++;
if (!client-dispatchFunc ||
client-dispatchFunc(client, msg,
client-dispatchOpaque) 0) {
virNetMessageFree(msg);
client-wantClose = true;
+ client-refs--;
return;
}
}
Again, this seems wrong and the ref should be removed in the success
case here too. Before I spent time to figure out how the refcounting
is supposed to work here I just report it and hope that Dan can easily
fix this.
Okay, after some discussion on IRC with Michal and Eric and further
testing I think that the ref counting is okay here.
virNetClientNew adds the second ref because the free callback
(virNetClientEventFree) passed to virNetSocketAddIOCallback will call
virNetClientFree.
Another observation is that virNetClientFree calls
virNetSocketRemoveIOCallback when the last ref was removed. Actually
that's pointless because virNetSocketRemoveIOCallback might indirectly
removed the last ref. So this looks like an ordering problem. The
ordering gets fixed by calling virNetClientClose before
virNetClientFree, because virNetClientClose calls
virNetSocketRemoveIOCallback. Another problem is that
virNetSocketRemoveIOCallback only indirectly results in a call to
virNetClientFree, because the event loop will call virNetClientFree
when removing callbacks marked for deletion --
virNetSocketRemoveIOCallback does this marking.
The memory leak I saw was due to virsh calling
virEventRegisterDefaultImpl, but not calling virEventRunDefaultImpl.
Because the event loop is initialized, the call to
virNetSocketAddIOCallback in virNetClientNew succeeds. But virsh not
driving the event loop results in not removing callbacks that were
marked for deletion. Finally this leaks the virNetClientPtr using in
the remote driver. I used a virsh -c qemu:///system to test.
I was able fix this by calling virEventRunDefaultImpl after
virConnectClose in virsh. But I don't think that this is the correct
general solution.
To me the general problem seems to be the entanglement between the
virNetClientPtr refcounting and the event loop. I'm not sure how to
improve this in general. Maybe should have a thread calling
virEventRunDefaultImpl as the comment on virEventRunDefaultImpl
suggest.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: fix build failure due to change in virDomainGraphicsDef
This failure was introduced by commit dacee3d, which removed
listenAddr from the unions in virDomainGraphicsDef in favor of putting
it in the address attribute of virDomainGraphicsListenDef.
---
src/libxl/libxl_conf.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b74a4b1..09f3be8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -651,6 +651,7 @@ libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainDefPtr
def,
virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb)
{
int port;
+const char *listenAddr;
switch (l_vfb-type) {
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
@@ -682,11 +683,11 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainDefPtr def,
}
x_vfb-vncdisplay = l_vfb-data.vnc.port - LIBXL_VNC_PORT_MIN;
-if (l_vfb-data.vnc.listenAddr) {
+listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
+if (listenAddr) {
/* libxl_device_vfb_init() does strdup(127.0.0.1) */
free(x_vfb-vnclisten);
-if ((x_vfb-vnclisten =
-strdup(l_vfb-data.vnc.listenAddr)) == NULL) {
+if ((x_vfb-vnclisten = strdup(listenAddr)) == NULL) {
virReportOOMError();
return -1;
}
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 2/3] save: let qemu driver manipulate save files
On 07/22/2011 12:21 AM, Eric Blake wrote:
The goal here is that save-image-dumpxml fed back to save
image-define without changing the save file; anywhere that
this is not the case is probably a bug in domain_conf.c.
* src/qemu/qemu_driver.c (qemuDomainSaveImageGetXMLDesc)
(qemuDomainSaveImageDefineXML): New functions.
(qemuDomainSaveImageOpen): Add parameter.
(qemuDomainRestoreFlags, qemuDomainObjRestore): Adjust clients.
---
v3: short cut exit with special value if no edits need to be made
src/qemu/qemu_driver.c | 120 ---
1 files changed, 112 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b1df6c..f1a4e8a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3706,29 +3706,32 @@ cleanup:
return ret;
}
-static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+/* Return -1 on failure, -2 if edit was specified but xmlin does not
+ * represent any changes, and opened fd on all other success. */
+static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
qemuDomainSaveImageOpen(struct qemud_driver *driver,
const char *path,
virDomainDefPtr *ret_def,
struct qemud_save_header *ret_header,
bool bypass_cache, virFileDirectFdPtr *directFd,
-const char *xmlin)
+const char *xmlin, bool edit)
{
int fd;
struct qemud_save_header header;
char *xml = NULL;
virDomainDefPtr def = NULL;
-int directFlag = 0;
+int oflags = edit ? O_RDWR : O_RDONLY;
if (bypass_cache) {
-directFlag = virFileDirectFdFlag();
+int directFlag = virFileDirectFdFlag();
if (directFlag 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
_(bypass cache unsupported by this system));
goto error;
}
+oflags |= directFlag;
}
-if ((fd = virFileOpenAs(path, O_RDONLY | directFlag, 0,
+if ((fd = virFileOpenAs(path, oflags, 0,
getuid(), getgid(), 0)) 0) {
if ((fd != -EACCES fd != -EPERM) ||
driver-user == getuid()) {
@@ -3739,7 +3742,7 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
/* Opening as root failed, but qemu runs as a different user
* that might have better luck. */
-if ((fd = virFileOpenAs(path, O_RDONLY | directFlag, 0,
+if ((fd = virFileOpenAs(path, oflags, 0,
driver-user, driver-group,
VIR_FILE_OPEN_AS_UID)) 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
@@ -3791,6 +3794,15 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
goto error;
}
+if (edit STREQ(xml, xmlin)) {
+VIR_FREE(xml);
+if (VIR_CLOSE(fd) 0) {
+virReportSystemError(errno, _(cannot close file: %s), path);
+goto error;
+}
+return -2;
+}
So unchanged is indicated with a 0 return code, but that only
happens when edit == true, and only one of the callers does that (and
properly handles the special case).
+
/* Create a domain from this XML */
if (!(def = virDomainDefParseString(driver-caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
@@ -3949,7 +3961,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
fd = qemuDomainSaveImageOpen(driver, path,def,header,
(flags VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
-directFd, dxml);
+directFd, dxml, false);
if (fd 0)
goto cleanup;
@@ -3995,6 +4007,96 @@ qemuDomainRestore(virConnectPtr conn,
return qemuDomainRestoreFlags(conn, path, NULL, 0);
}
+static char *
+qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
+ unsigned int flags)
+{
+struct qemud_driver *driver = conn-privateData;
+char *ret = NULL;
+virDomainDefPtr def = NULL;
+int fd = -1;
+struct qemud_save_header header;
+
+/* We only take subset of virDomainDefFormat flags. */
+virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
+
+qemuDriverLock(driver);
+
+fd = qemuDomainSaveImageOpen(driver, path,def,header, false, NULL,
+ NULL, false);
+
+if (fd 0)
+goto cleanup;
+
+ret = qemuDomainDefFormatXML(driver, def, flags);
+
+cleanup:
+virDomainDefFree(def);
+VIR_FORCE_CLOSE(fd);
+qemuDriverUnlock(driver);
+return ret;
+}
+
+static int
+qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
+ const char *dxml, unsigned int flags)
+{
+struct qemud_driver *driver = conn-privateData;
+int ret = -1;
+virDomainDefPtr def = NULL;
+int fd = -1;
+struct qemud_save_header header;
+char *xml
Re: [libvirt] [PATCH] libxl: fix build failure due to change in virDomainGraphicsDef
On 07/28/2011 12:41 PM, Laine Stump wrote: This failure was introduced by commit dacee3d, which removed listenAddr from the unions in virDomainGraphicsDef in favor of putting it in the address attribute of virDomainGraphicsListenDef. --- src/libxl/libxl_conf.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC: PATCHv3 3/3] save: generate idempotent inactive xml for running domain
On 07/22/2011 12:21 AM, Eric Blake wrote:
Noticed by comparing the xml generated by virDomainSave with the
xml produced by reparsing and redumping that xml.
* src/conf/domain_conf.c (virDomainDeviceInfoIsSet): Add
parameter, and update all callers. Make static.
(virDomainNetDefFormat): Skip generated ifname.
* src/conf/domain_conf.h (virDomainDeviceInfoIsSet): Delete.
* src/libvirt_private.syms (domain_conf.h): Update.
---
Sending this now, to get review started, but I still have some
more fixing to do - right now, active domains still include:
+seclabel type='dynamic' model='selinux' relabel='yes'/
which is not present on reparse, but I'm too tired to find out why.
I know the feeling :-)
So does it turn out that this is important, or not?
src/conf/domain_conf.c | 26 --
src/conf/domain_conf.h |1 -
src/libvirt_private.syms |1 -
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 919a75a..52e8ada 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1388,11 +1388,12 @@ int virDomainDeviceVirtioSerialAddressIsValid(
}
-int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info)
+static int
+virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags)
{
if (info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
return 1;
-if (info-alias)
+if (info-alias !(flags VIR_DOMAIN_XML_INACTIVE))
return 1;
return 0;
}
@@ -8580,7 +8581,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
break;
}
-if (virDomainDeviceInfoIsSet(def-info)) {
+if (virDomainDeviceInfoIsSet(def-info, flags)) {
virBufferAddLit(buf, \n);
if (virDomainDeviceInfoFormat(buf,def-info, flags) 0)
return -1;
@@ -8806,9 +8807,14 @@ virDomainNetDefFormat(virBufferPtr buf,
break;
}
-if (def-ifname)
+
+if (def-ifname
+!((flags VIR_DOMAIN_XML_INACTIVE)
+ (STRPREFIX(def-ifname, vnet {
+/* Skip auto-generated target names for inactive config. */
It's kind of bothersome that use of this magic device name prefix isn't
self-contained in domain_conf.c (or somewhere else). Perhaps the string
could be defined in domain_conf.h, then used here and in qemu_command.c
(is it used any place else?).
Aside from that bit of ugliness, ACK. (And I could live with this. at
least temporarily, as well, although making all the places work off a
single string constant might hypothetically save someone lots of
frustration in some uncharted future.)
virBufferEscapeString(buf, target dev='%s'/\n,
def-ifname);
+}
if (def-model) {
virBufferEscapeString(buf, model type='%s'/\n,
def-model);
@@ -9041,7 +9047,7 @@ virDomainChrDefFormat(virBufferPtr buf,
break;
}
-if (virDomainDeviceInfoIsSet(def-info)) {
+if (virDomainDeviceInfoIsSet(def-info, flags)) {
if (virDomainDeviceInfoFormat(buf,def-info, flags) 0)
return -1;
}
@@ -9069,7 +9075,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, smartcard mode='%s', mode);
switch (def-type) {
case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
-if (!virDomainDeviceInfoIsSet(def-info)) {
+if (!virDomainDeviceInfoIsSet(def-info, flags)) {
virBufferAddLit(buf, /\n);
return 0;
}
@@ -9119,7 +9125,7 @@ virDomainSoundDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, sound model='%s',
model);
-if (virDomainDeviceInfoIsSet(def-info)) {
+if (virDomainDeviceInfoIsSet(def-info, flags)) {
virBufferAddLit(buf, \n);
if (virDomainDeviceInfoFormat(buf,def-info, flags) 0)
return -1;
@@ -9148,7 +9154,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, memballoon model='%s',
model);
-if (virDomainDeviceInfoIsSet(def-info)) {
+if (virDomainDeviceInfoIsSet(def-info, flags)) {
virBufferAddLit(buf, \n);
if (virDomainDeviceInfoFormat(buf,def-info, flags) 0)
return -1;
@@ -9197,7 +9203,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, watchdog model='%s' action='%s',
model, action);
-if (virDomainDeviceInfoIsSet(def-info)) {
+if (virDomainDeviceInfoIsSet(def-info, flags)) {
virBufferAddLit(buf, \n);
if (virDomainDeviceInfoFormat(buf,def-info, flags) 0)
return -1;
@@ -9280,7 +9286,7 @@ virDomainInputDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, input type='%s' bus='%s',
type, bus);
-if (virDomainDeviceInfoIsSet(def-info)) {
+if (virDomainDeviceInfoIsSet(def-info, flags)) {
virBufferAddLit(buf, \n);
if
Re: [libvirt] [PATCH] libxl: fix build failure due to change in virDomainGraphicsDef
On 07/28/2011 02:49 PM, Eric Blake wrote: On 07/28/2011 12:41 PM, Laine Stump wrote: This failure was introduced by commit dacee3d, which removed listenAddr from the unions in virDomainGraphicsDef in favor of putting it in the address attribute of virDomainGraphicsListenDef. --- src/libxl/libxl_conf.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) ACK. Pushed. Sorry for the inconvenience. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt-0.9.3 and qemu-0.14.1
Hello, I'm not sure if this has been reported or fixed, but ... ~~~ SNIP ~~~ 21:02:59.981: 13171: info : libvirt version: 0.9.3 21:02:59.981: 13171: error : virCommandWait:1957 : internal error Child process (LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root /usr/bin/qemu-kvm -help) status unexpected: exit status 1 21:02:59.986: 13171: error : virCommandWait:1957 : internal error Child process (LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root /usr/bin/qemu-kvm -help) status unexpected: exit status 1 21:02:59.988: 13171: error : virDomainDefParseXML:6069 : unknown OS type hvm ~~~ SNIP ~~~ qemu-0.14.1 and libvirt-0.9.3 What am I missing here? Thanks, Z. -- Zdenek Styblik email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] maint: add missing copyright notices
I went with the shorter license notice used by src/libvirt.c, rather than spelling out the full LGPLv2+ clause into each of these files. * configure.ac: Declare copyright. * all Makefile.am: Likewise. --- Noticed while reviewing Matthias' patch. Almost trivial enough to push in isolation, but I'll wait for the review. Makefile.am |3 +++ configure.ac|3 +++ daemon/Makefile.am |3 +++ docs/Makefile.am|4 docs/schemas/Makefile.am|3 ++- examples/apparmor/Makefile.am |3 +++ examples/domain-events/events-c/Makefile.am |3 +++ examples/dominfo/Makefile.am|2 ++ examples/domsuspend/Makefile.am |2 ++ examples/hellolibvirt/Makefile.am |3 +++ examples/openauth/Makefile.am |3 +++ examples/python/Makefile.am |3 +++ examples/systemtap/Makefile.am |2 ++ examples/xml/nwfilter/Makefile.am |2 ++ include/Makefile.am |4 include/libvirt/Makefile.am |3 +++ python/Makefile.am |3 +++ python/tests/Makefile.am|3 +++ src/Makefile.am |3 +++ tests/Makefile.am |3 +++ tools/Makefile.am |2 ++ 21 files changed, 59 insertions(+), 1 deletions(-) diff --git a/Makefile.am b/Makefile.am index 49e42bf..75cff8d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,5 +1,8 @@ ## Process this file with automake to produce Makefile.in +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software + LCOV = lcov GENHTML = genhtml diff --git a/configure.ac b/configure.ac index ac34efc..36037e5 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,8 @@ dnl Process this file with autoconf to produce a configure script. +dnl Copyright (C) 2005-2011 Red Hat, Inc. +dnl See COPYING.LIB for the License of this software + AC_INIT([libvirt], [0.9.3], [libvir-list@redhat.com], [], [http://libvirt.org]) AC_CONFIG_SRCDIR([src/libvirt.c]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 63c731e..65ac8e9 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,5 +1,8 @@ ## Process this file with automake to produce Makefile.in +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software + CLEANFILES = DAEMON_GENERATED = \ diff --git a/docs/Makefile.am b/docs/Makefile.am index de649fe..50a199f 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -1,4 +1,8 @@ ## Process this file with automake to produce Makefile.in + +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software + SUBDIRS= schemas PERL = perl diff --git a/docs/schemas/Makefile.am b/docs/schemas/Makefile.am index 75a0e73..596c207 100644 --- a/docs/schemas/Makefile.am +++ b/docs/schemas/Makefile.am @@ -1,4 +1,5 @@ - +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software schemadir = $(pkgdatadir)/schemas schema_DATA = \ diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index b72bbe1..0bc66ac 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -1,3 +1,6 @@ +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software + EXTRA_DIST=\ TEMPLATE\ libvirt-qemu\ diff --git a/examples/domain-events/events-c/Makefile.am b/examples/domain-events/events-c/Makefile.am index 176892b..2bcbca0 100644 --- a/examples/domain-events/events-c/Makefile.am +++ b/examples/domain-events/events-c/Makefile.am @@ -1,3 +1,6 @@ +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software + INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include \ -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib noinst_PROGRAMS = event-test diff --git a/examples/dominfo/Makefile.am b/examples/dominfo/Makefile.am index 678de68..07982b0 100644 --- a/examples/dominfo/Makefile.am +++ b/examples/dominfo/Makefile.am @@ -1,3 +1,5 @@ +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -I@srcdir@/include LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \ diff --git a/examples/domsuspend/Makefile.am b/examples/domsuspend/Makefile.am index 2c277a4..fc5e931 100644 --- a/examples/domsuspend/Makefile.am +++ b/examples/domsuspend/Makefile.am @@ -1,3 +1,5 @@ +## Copyright (C) 2005-2011 Red Hat, Inc. +## See COPYING.LIB for the License of this software
[libvirt] [PATCHv2 2/2] freebsd: Fix build problem due to picking up the wrong libvirt.h
From: Matthias Bolte matthias.bo...@googlemail.com
Gettext annoyingly modifies CPPFLAGS in-place, putting
-I/usr/local/include into the search patch if libintl headers
must be used from that location. But since we must support
automake 1.9.6 which lacks AM_CPPFLAGS, and since CPPFLAGS is used
prior to INCLUDES, this means that the build picks up the _old_
installed libvirt.h in priority to the in-tree version, leading
to all sorts of weird build failures on FreeBSD.
Fix this by teaching configure to undo gettext's actions, but
to keep any changes required by gettext at the end of INCLUDES
after all in-tree locations are used first. Also requires
adding a wrapper Makefile.am and making gnulib-tool create
just gnulib.mk files during the bootstrap process.
---
v1 is here:
https://www.redhat.com/archives/libvir-list/2011-July/msg01984.html
v2: update bootstrap.conf and gnulib/*/Makefile.am to fix gnulib
compilation, update .gitignore to allow committing new files.
.gitignore |6 +-
bootstrap.conf |5 ++---
configure.ac | 22 ++
daemon/Makefile.am | 17 ++---
gnulib/lib/Makefile.am |8
gnulib/tests/Makefile.am |8
python/Makefile.am |3 ++-
src/Makefile.am |5 +++--
tests/Makefile.am|3 ++-
tools/Makefile.am| 13 -
10 files changed, 70 insertions(+), 20 deletions(-)
create mode 100644 gnulib/lib/Makefile.am
create mode 100644 gnulib/tests/Makefile.am
diff --git a/.gitignore b/.gitignore
index dd64ca5..60220ad 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,7 +36,9 @@
/configure.lineno
/daemon/*_dispatch.h
/docs/hvsupport.html.in
-/gnulib/
+/gnulib/lib/*
+/gnulib/m4/*
+/gnulib/tests/*
/libtool
/libvirt-*.tar.gz
/libvirt-[0-9]*
@@ -74,6 +76,8 @@ results.log
stamp-h
stamp-h.in
stamp-h1
+!/gnulib/lib/Makefile.am
+!/gnulib/tests/Makefile.am
!/m4/virt-*.m4
!/po/*.po
!/po/POTFILES.in
diff --git a/bootstrap.conf b/bootstrap.conf
index 3b105b1..7882886 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -161,7 +161,6 @@ gnulib_name=libgnu
m4_base=gnulib/m4
source_base=gnulib/lib
tests_base=gnulib/tests
-gnulib_mk=Makefile.am
gnulib_tool_option_extras=\
--lgpl=2\
--with-tests\
@@ -203,9 +202,9 @@ gnulib_extra_files=
bootstrap_epilogue()
{
- # Change paths in gnulib/tests/Makefile.am from ../../.. to ../..,
+ # Change paths in gnulib/tests/gnulib.mk from ../../.. to ../..,
# then ensure that gnulib/tests/Makefile.in is up-to-date.
- m=gnulib/tests/Makefile.am
+ m=gnulib/tests/gnulib.mk
sed 's,\.\./\.\./\.\.,../..,g' $m $m-t
mv -f $m-t $m
${AUTOMAKE-automake} gnulib/tests/Makefile
diff --git a/configure.ac b/configure.ac
index 36037e5..34bc1fe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2058,8 +2058,30 @@ dnl Enable building libvirtd?
AM_CONDITIONAL([WITH_LIBVIRTD],[test x$with_libvirtd = xyes])
dnl Check for gettext - don't go any newer than what RHEL 5 supports
+dnl
+dnl save and restore CPPFLAGS around gettext check as the internal iconv
+dnl check might leave -I/usr/local/include in CPPFLAGS on FreeBSD resulting
+dnl in the build picking up previously installed libvirt/libvirt.h instead
+dnl of the correct one from the source tree.
+dnl compute the difference between save_CPPFLAGS and CPPFLAGS and append it
+dnl to INCLUDES in order to preserve changes made by gettext but in a place
+dnl that does not break the build
+save_CPPFLAGS=$CPPFLAGS
AM_GNU_GETTEXT_VERSION([0.17])
AM_GNU_GETTEXT([external])
+GETTEXT_CPPFLAGS=
+if test x$save_CPPFLAGS != x$CPPFLAGS; then
+ set dummy $CPPFLAGS; shift
+ for var
+ do
+ case $var in
+ $save_CPPFLAGS ) ;;
+ *) GETTEXT_CPPFLAGS=$GETTEXT_CPPFLAGS $var ;;
+ esac
+ done
+fi
+CPPFLAGS=$save_CPPFLAGS
+AC_SUBST([GETTEXT_CPPFLAGS])
ALL_LINGUAS=`cd $srcdir/po /dev/null ls *.po | sed 's+\.po$++'`
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 65ac8e9..690bf85 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -3,6 +3,16 @@
## Copyright (C) 2005-2011 Red Hat, Inc.
## See COPYING.LIB for the License of this software
+INCLUDES = \
+ -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
+ -I$(top_srcdir)/include -I$(top_builddir)/include \
+ -I$(top_srcdir)/src \
+ -I$(top_srcdir)/src/util \
+ -I$(top_srcdir)/src/conf \
+ -I$(top_srcdir)/src/rpc \
+ -I$(top_srcdir)/src/remote \
+ $(GETTEXT_CPPFLAGS)
+
CLEANFILES =
DAEMON_GENERATED = \
@@ -82,13 +92,6 @@ libvirtd_SOURCES = $(DAEMON_SOURCES)
#-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L
libvirtd_CFLAGS = \
- -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
- -I$(top_srcdir)/include -I$(top_builddir)/include \
- -I$(top_srcdir)/src \
- -I$(top_srcdir)/src/util \
- -I$(top_srcdir)/src/conf \
- -I$(top_srcdir)/src/rpc \
-
Re: [libvirt] [PATCH] freebsd: Fix build problem due to picking up the wrong libvirt.h
On 07/28/2011 07:51 AM, Eric Blake wrote: Is there any option in gnulib that would allow to inject GETTEXT_CPPFLAGS into the gnulib makefiles, or any other possibility to fix this? Fortunately, I know how to fix that - it involves changing our gnulib-tool to spit out gnulib.mk to be included from a wrapper Makefile.am that we manage, rather than our current practice of letting gnulib-tool spit out the complete Makefile.am (coreutils.git does the same thing). I attached a preliminary patch. I'll submit a v2 based on your patch shortly. https://www.redhat.com/archives/libvir-list/2011-July/msg02019.html -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] xen: cleanup callback struct
On 07/21/2011 05:39 PM, Eric Blake wrote: Using C99 initializers and xen-specific prefixes will make it so that future patches are less likely to add callback members to the xenUnifiedDriver struct, since the goal is to get rid of the callback struct in the first place. * src/xen/xen_driver.h (xenUnifiedDriver): Rename all struct members, to make it obvious which ones are still in use. * src/xen/xen_driver.c: Update all callers. * src/xen/xen_hypervisor.c (xenHypervisorDriver): Rewrite with C99 initializers. * src/xen/xend_internal.c (xenDaemonDriver): Likewise. * src/xen/xs_internal.c (xenStoreDriver): Likewise. * src/xen/xm_internal.c (xenXMDriver): Likewise. * src/xen/xen_inotify.c (xenInotifyDriver): Likewise. Completely Mechanical. I didn't see any discrepencies (looking with ediff). ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] xen: reduce callback special cases
On 07/21/2011 05:39 PM, Eric Blake wrote: The callback struct is great when iterating through several possibilities, but when calling a known callback, it's just overhead. We can make the direct call in those cases. * src/xen/xen_driver.c (xenUnifiedOpen, xenUnifiedDomainSuspend) (xenUnifiedDomainResume, xenUnifiedDomainDestroyFlags): Make direct calls instead of going through callback. Again, purely mechanical, and no problems noticed in the conversion. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-0.9.3 and qemu-0.14.1
On 07/28/11 21:24, Zdenek Styblik wrote: Hello, I'm not sure if this has been reported or fixed, but ... ~~~ SNIP ~~~ 21:02:59.981: 13171: info : libvirt version: 0.9.3 21:02:59.981: 13171: error : virCommandWait:1957 : internal error Child process (LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root /usr/bin/qemu-kvm -help) status unexpected: exit status 1 21:02:59.986: 13171: error : virCommandWait:1957 : internal error Child process (LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root /usr/bin/qemu-kvm -help) status unexpected: exit status 1 21:02:59.988: 13171: error : virDomainDefParseXML:6069 : unknown OS type hvm ~~~ SNIP ~~~ qemu-0.14.1 and libvirt-0.9.3 What am I missing here? Thanks, Z. Never mind. I had to write it somewhere to realize the problem and get it fixed. :) Have a nice day, Z. -- Zdenek Styblik email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] xen: make direct call when there is only one subdriver
On 07/21/2011 05:39 PM, Eric Blake wrote:
No need to use a for loop if we know there is exactly one client.
* src/xen/xen_driver.c (xenUnifiedClose): Call close
unconditionally, to match xenUnifiedOpen.
(xenUnifiedNodeGetInfo, xenUnifiedDomainCreateXML)
(xenUnifiedDomainSave, xenUnifiedDomainRestore)
(xenUnifiedDomainCoreDump, xenUnifiedDomainUpdateDeviceFlags):
Make direct call to lone implementation.
* src/xen/xend_internal.h (xenDaemonDomainCoreDump)
(xenDaemonUpdateDeviceFlags, xenDaemonCreateXML): Add prototypes.
* src/xen/xend_internal.c (xenDaemonDomainCoreDump)
(xenDaemonUpdateDeviceFlags, xenDaemonCreateXML): Export.
---
src/xen/xen_driver.c| 61 --
src/xen/xend_internal.c |6 ++--
src/xen/xend_internal.h |6
3 files changed, 25 insertions(+), 48 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 2bad8c4..b7122f0 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -414,7 +414,8 @@ fail:
clean:
VIR_DEBUG(Failed to activate a mandatory sub-driver);
for (i = 0 ; i XEN_UNIFIED_NR_DRIVERS ; i++)
-if (priv-opened[i]) drivers[i]-xenClose(conn);
+if (priv-opened[i])
+drivers[i]-xenClose(conn);
I only see whitespace change here. Was there supposed to be more?
virMutexDestroy(priv-lock);
VIR_FREE(priv);
conn-privateData = NULL;
@@ -434,8 +435,8 @@ xenUnifiedClose (virConnectPtr conn)
virDomainEventCallbackListFree(priv-domainEventCallbacks);
for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenClose)
-(void) drivers[i]-xenClose (conn);
+if (priv-opened[i])
+drivers[i]-xenClose(conn);
I guess the logic here is that if opened[i] is true, there must have
been a xenOpen(), and if there was a xenOpen() there must be a xenClose()?
virMutexDestroy(priv-lock);
VIR_FREE(conn-privateData);
@@ -537,14 +538,9 @@ static int
xenUnifiedNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info)
{
GET_PRIVATE(conn);
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenNodeGetInfo
-drivers[i]-xenNodeGetInfo (conn, info) == 0)
-return 0;
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
+return xenDaemonNodeGetInfo(conn, info);
return -1;
For all of these, is it that you've determined by examining all of the
driver info structs that only one driver has a particular callback? Can
we guarantee that will continue to be the case in the future?
Conditional ACK, based on satisfactory answers to these questions...
}
@@ -621,15 +617,9 @@ xenUnifiedDomainCreateXML (virConnectPtr conn,
const char *xmlDesc, unsigned int flags)
{
GET_PRIVATE(conn);
-int i;
-virDomainPtr ret;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i] drivers[i]-xenDomainCreateXML) {
-ret = drivers[i]-xenDomainCreateXML (conn, xmlDesc, flags);
-if (ret) return ret;
-}
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
+return xenDaemonCreateXML(conn, xmlDesc, flags);
return NULL;
}
@@ -1055,14 +1045,9 @@ static int
xenUnifiedDomainSave (virDomainPtr dom, const char *to)
{
GET_PRIVATE(dom-conn);
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenDomainSave
-drivers[i]-xenDomainSave (dom, to) == 0)
-return 0;
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
+return xenDaemonDomainSave(dom, to);
return -1;
}
@@ -1070,14 +1055,9 @@ static int
xenUnifiedDomainRestore (virConnectPtr conn, const char *from)
{
GET_PRIVATE(conn);
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenDomainRestore
-drivers[i]-xenDomainRestore (conn, from) == 0)
-return 0;
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
+return xenDaemonDomainRestore(conn, from);
return -1;
}
@@ -1085,14 +1065,9 @@ static int
xenUnifiedDomainCoreDump (virDomainPtr dom, const char *to, unsigned int
flags)
{
GET_PRIVATE(dom-conn);
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
-drivers[i]-xenDomainCoreDump
-drivers[i]-xenDomainCoreDump (dom, to, flags) == 0)
-return 0;
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
+return xenDaemonDomainCoreDump(dom, to, flags);
return -1;
}
@@ -1630,13 +1605,9 @@ xenUnifiedDomainUpdateDeviceFlags (virDomainPtr dom,
const char *xml,
unsigned int flags)
{
GET_PRIVATE(dom-conn);
-int i;
-
-for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]
Re: [libvirt] [PATCH 4/4] xen: drop unused callbacks
On 07/21/2011 05:39 PM, Eric Blake wrote: Found by: for f in $(sed -n 's/.*Drv[^ ]* \([^;]*\);.*/\1/p' src/xen/xen_driver.h) do git grep \(\.\|-\)$f\b src/xen done | cat and looking through the resulting list to see which callback struct members are still necessary. Sure. Why not? Simple is good. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] support qemuMonitorSend() to be called at the same time
On 07/28/2011 03:13 AM, Wen Congyang wrote:
Current, we support run sync job and async job at the same time. It means that
the
monitor commands for two jobs can be run in any order.
In the function qemuDomainObjEnterMonitorInternal():
if (priv-job.active == QEMU_JOB_NONE priv-job.asyncJob) {
if (qemuDomainObjBeginNestedJob(driver, obj) 0)
We check whether the caller is an async job by priv-job.active and
priv-job.asynJob. But when an async job is running, priv-job.active
is QEMU_JOB_NONE if no sync job is running, or priv-job.active is not
QEMU_JOB_NONE if a sync job is running. So we cannot check whether the
caller is an async job in the function qemuDomainObjEnterMonitorInternal().
Unfortunately, if sync job and async job are running at the same time, we may
try to send monitor command at the same time in two threads. It's very
dangerous, and it will cause libvirtd crashed.
Thanks for the analysis. I think you are spot on as to the problem's
root cause - we are not properly serializing access to the monitor when
mixing a sync and an async job.
However, I'm not yet convinced that adding yet another new condvar is
the solution. Rather, your idea of adding
qemuDomainObjEnterMonitorWithDriverAsync() might be the way to go. And
as for existing functions that can be used either by the async job or by
a sync job (qemuProcessStopCPUs), I think that merely means that
qemuDomainObjEnterMonitorWithDriverAsync is passed either
QEMU_ASYNC_JOB_NONE (sync job request) or the current async job, and
that qemuDomainObjEnterMonitorWithDriver becomes shorthand for
qemuDomainObjEnterMonitorWithDriver(,QEMU_ASYNC_JOB_NONE).
I'll try a patch along those lines, but may end up going with yours
after all.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] maint: add missing copyright notices
On 07/28/2011 03:32 PM, Eric Blake wrote: I went with the shorter license notice used by src/libvirt.c, rather than spelling out the full LGPLv2+ clause into each of these files. * configure.ac: Declare copyright. * all Makefile.am: Likewise. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] xen: make direct call when there is only one subdriver
On 07/28/2011 02:00 PM, Laine Stump wrote:
On 07/21/2011 05:39 PM, Eric Blake wrote:
No need to use a for loop if we know there is exactly one client.
VIR_DEBUG(Failed to activate a mandatory sub-driver);
for (i = 0 ; i XEN_UNIFIED_NR_DRIVERS ; i++)
- if (priv-opened[i]) drivers[i]-xenClose(conn);
+ if (priv-opened[i])
+ drivers[i]-xenClose(conn);
I only see whitespace change here. Was there supposed to be more?
This one was whitespace only (our style being that if() and its embedded
statements should be separated lines).
virMutexDestroy(priv-lock);
VIR_FREE(priv);
conn-privateData = NULL;
@@ -434,8 +435,8 @@ xenUnifiedClose (virConnectPtr conn)
virDomainEventCallbackListFree(priv-domainEventCallbacks);
for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
- if (priv-opened[i] drivers[i]-xenClose)
- (void) drivers[i]-xenClose (conn);
+ if (priv-opened[i])
+ drivers[i]-xenClose(conn);
I guess the logic here is that if opened[i] is true, there must have
been a xenOpen(), and if there was a xenOpen() there must be a xenClose()?
Correct. I will also squash in a variant of this hunk from patch 4/4 to
make it more obvious that xenOpen and xenClose callbacks must be
non-NULL (xen-inotify was the best example of using only those two
callbacks):
struct xenUnifiedDriver {
-virDrvOpen xenOpen;
-virDrvClose xenClose;
+virDrvClose xenClose; /* Only mandatory callback; all others may be
NULL */
virMutexDestroy(priv-lock);
VIR_FREE(conn-privateData);
@@ -537,14 +538,9 @@ static int
xenUnifiedNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info)
{
GET_PRIVATE(conn);
- int i;
-
- for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
- if (priv-opened[i]
- drivers[i]-xenNodeGetInfo
- drivers[i]-xenNodeGetInfo (conn, info) == 0)
- return 0;
+ if (priv-opened[XEN_UNIFIED_XEND_OFFSET])
+ return xenDaemonNodeGetInfo(conn, info);
return -1;
For all of these, is it that you've determined by examining all of the
driver info structs that only one driver has a particular callback? Can
we guarantee that will continue to be the case in the future?
The guarantee that there is only one driver using the callback was made
by the formula in the commit comment in patch 4:
for f in $(sed -n 's/.*Drv[^ ]* \([^;]*\);.*/\1/p' src/xen/xen_driver.h)
do
git grep \(\.\|-\)$f\b src/xen
done | cat
and looking through the resulting list to see which callback struct
members are used exactly once.
I'll update the commit message accordingly.
The guarantee that this will continue to be the case in the future:
well, after this patch, there are 0 uses of these particular callbacks,
so patch 4 removes the callback member from the struct. If the struct
doesn't have a callback member, then new code can't accidentally add
another callback :)
Conditional ACK, based on satisfactory answers to these questions...
I hope that was satisfactory, because I'm pushing as soon as I also
answer your comments to patch 4 :)
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] xen: make direct call when there is only one subdriver
On 07/28/2011 04:34 PM, Eric Blake wrote: On 07/28/2011 02:00 PM, Laine Stump wrote: Conditional ACK, based on satisfactory answers to these questions... I hope that was satisfactory, because I'm pushing as soon as I also answer your comments to patch 4 :) Yep. I'm happy. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] xen: drop unused callbacks
On 07/28/2011 02:03 PM, Laine Stump wrote: On 07/21/2011 05:39 PM, Eric Blake wrote: Found by: for f in $(sed -n 's/.*Drv[^ ]* \([^;]*\);.*/\1/p' src/xen/xen_driver.h) do git grep \(\.\|-\)$f\b src/xen done | cat and looking through the resulting list to see which callback struct members are still necessary. /me glad I wrote this in the commit message, as it wasn't trivial to come up with Sure. Why not? Simple is good. ACK. OK, series pushed, with patch 3 amended as described in my reply. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] maint: add missing copyright notices
On 07/28/2011 02:24 PM, Laine Stump wrote: On 07/28/2011 03:32 PM, Eric Blake wrote: I went with the shorter license notice used by src/libvirt.c, rather than spelling out the full LGPLv2+ clause into each of these files. * configure.ac: Declare copyright. * all Makefile.am: Likewise. ACK. Patch 1/2 now pushed; I'll wait for a report on patch 2/2 actually being tested on FreeBSD before doing anything further with that, though. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/3] save: support qemu modifying xml on domain save/restore
On 07/28/2011 12:00 PM, Laine Stump wrote: --- v3: change virDomainSave to always output minimal information, but with fixed padding added, so that save file modification will always be more likely to succeed, always be more likely. Heh. I guess dropping always would make that sentence more believable. Looking at this problem from the outside, it seems that if we wanted a 100% reliable solution, we would need to introduce the idea of a linked header, which can be continued at the end of the file (of course that wouldn't work if there are ever cases where the file is being read from a pipe, and we can't seek, and it's entirely possible that 1024 is always enough extra to ensure everything works). We hand the file over to qemu after seeking to the end of our header, and I don't think qemu tolerates garbage at the end of its saved state files (qemu only reads in the saved state as if by a pipe; in reality the saved state file is the same as what gets sent over a socket during migration, which really is a one-pass non-seeking algorithm). So a split header won't really work. But hopefully there's not too many ABI compatible changes that you can make that significantly increase the size of the XML. And in the worst case, you can always fall back to: virsh restore file --xml alternate in the case where virsh save-image-define file alternate complains about a too-large alternate. ACK. Thanks; pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?
Hi Eric,
On Wed, Jul 27, 2011 at 02:52:46PM -0600, Eric Blake wrote:
[..snip..]
Pasting from that URL gave awkward results below; can you address my
comments below, then post a v2 as a proper patch against
libvirt.git?
Thanks for the review! New version attached.
[..snip..]
I don't like this part. It is not safe to pass %s as the pathname
through an additional round of shell parsing, since if the pathname
has anything that might be construed as shell metacharacters, the
parse will be destroyed. To some extent, that is already a
pre-existing bug (slightly mitigated by the fact that 'path' is
under libvirt's control, and should not be containing arbitrary
characters unless you passed odd directory choices to ./configure).
Would it make sense to pass such names through something like
g_shell_quote in instead of looking for troublesome characters? This
could be done using virBufferQuotedString? I'll post a patch for review
tomorrow.
Cheers,
-- Guido
From 0852938de266d8fc37c0558228177915e8b56031 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org
Date: Fri, 8 Jul 2011 21:07:29 +0200
Subject: [PATCH] Autodetect if the remote nc command supports the -q option
Based on a patch by Marc Deslauriers marc.deslauri...@ubuntu.com
RH: https://bugzilla.redhat.com/show_bug.cgi?id=562176
Ubuntu: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/517478
Debia: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=573172
---
src/rpc/virnetsocket.c | 23 ---
tests/virnetsockettest.c | 11 ++-
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index dcdc937..cba58c6 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -628,9 +628,26 @@ int virNetSocketNewConnectSSH(const char *nodename,
-e, none, NULL);
if (noVerify)
virCommandAddArgList(cmd, -o, StrictHostKeyChecking=no, NULL);
-virCommandAddArgList(cmd, nodename,
- netcat ? netcat : nc,
- -U, path, NULL);
+
+virCommandAddArgList(cmd, nodename, sh, -c, NULL);
+/*
+ * This ugly thing is a shell script to detect availability of
+ * the -q option for 'nc': debian and suse based distros need this
+ * flag to ensure the remote nc will exit on EOF, so it will go away
+ * when we close the connection tunnel. If it doesn't go away, subsequent
+ * connection attempts will hang.
+ *
+ * Fedora's 'nc' doesn't have this option, and defaults to the desired
+ * behavior.
+ */
+virCommandAddArgFormat(cmd,
+ 'if %s -q 21 | grep \requires an argument\ /dev/null 21; then
+ ARG=-q0;
+ fi;
+ %s $ARG -U %s',
+ netcat ? netcat : nc,
+ netcat ? netcat : nc,
+ path);
return virNetSocketNewConnectCommand(cmd, retsock);
}
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index e72b9a0..3816b3c 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -496,7 +496,7 @@ mymain(void)
struct testSSHData sshData1 = {
.nodename = somehost,
.path = /tmp/socket,
-.expectOut = somehost nc -U /tmp/socket\n,
+.expectOut = somehost sh -c 'if nc -q 21 | grep \requires an argument\ /dev/null 21; then ARG=-q0;fi;nc $ARG -U /tmp/socket'\n,
};
if (virtTestRun(SSH test 1, 1, testSocketSSH, sshData1) 0)
ret = -1;
@@ -509,7 +509,7 @@ mymain(void)
.noTTY = true,
.noVerify = false,
.path = /tmp/socket,
-.expectOut = -p 9000 -l fred -T -o BatchMode=yes -e none somehost netcat -U /tmp/socket\n,
+.expectOut = -p 9000 -l fred -T -o BatchMode=yes -e none somehost sh -c 'if netcat -q 21 | grep \requires an argument\ /dev/null 21; then ARG=-q0;fi;netcat $ARG -U /tmp/socket'\n,
};
if (virtTestRun(SSH test 2, 1, testSocketSSH, sshData2) 0)
ret = -1;
@@ -522,7 +522,7 @@ mymain(void)
.noTTY = false,
.noVerify = true,
.path = /tmp/socket,
-.expectOut = -p 9000 -l fred -o StrictHostKeyChecking=no somehost netcat -U /tmp/socket\n,
+.expectOut = -p 9000 -l fred -o StrictHostKeyChecking=no somehost sh -c 'if netcat -q 21 | grep \requires an argument\ /dev/null 21; then ARG=-q0;fi;netcat $ARG -U /tmp/socket'\n,
};
if (virtTestRun(SSH test 3, 1, testSocketSSH, sshData3) 0)
ret = -1;
@@ -538,7 +538,8 @@ mymain(void)
struct testSSHData sshData5 = {
.nodename = crashyhost,
.path = /tmp/socket,
-.expectOut = crashyhost nc -U /tmp/socket\n,
+.expectOut = crashyhost sh -c 'if nc -q 21 | grep \requires an argument\ /dev/null 21; then ARG=-q0;fi;nc $ARG -U /tmp/socket'\n,
+
.dieEarly = true,
};
if (virtTestRun(SSH test 5, 1, testSocketSSH, sshData5) 0)
@@ -549,7 +550,7 @@ mymain(void)
.path
Re: [libvirt] [virt-tools-list] ANNOUNCE: virt-manager 0.9.0 and virtinst 0.600.0 released
On Thu, Jul 28, 2011 at 12:31:10PM -0400, Cole Robinson thus spake: I'm happy to announce two new releases: virt-manager 0.9.0: virt-manager is a desktop application for managing KVM and Xen virtual machines via libvirt. virtinst 0.600.0: virtinst is a collection of command line tools for provisioning libvirt virtual machines, including virt-install and virt-clone. Thanks, Cole These have not been updated in the FreeBSD portstree, just yet, however I have submitted patches to update the current port. virt-manager: http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/159262 virtinst: http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/159261 Thanks! Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix crash when mixing sync and async monitor jobs
Currently, we attempt to run sync job and async job at the same time. It
means that the monitor commands for two jobs can be run in any order.
In the function qemuDomainObjEnterMonitorInternal():
if (priv-job.active == QEMU_JOB_NONE priv-job.asyncJob) {
if (qemuDomainObjBeginNestedJob(driver, obj) 0)
We check whether the caller is an async job by priv-job.active and
priv-job.asynJob. But when an async job is running, and a sync job is
also running at the time of the check, then priv-job.active is not
QEMU_JOB_NONE. So we cannot check whether the caller is an async job
in the function qemuDomainObjEnterMonitorInternal(), and must instead
put the burden on the caller to tell us when an async command wants
to do a nested job.
* src/qemu/THREADS.txt: Reflect new rules.
* src/qemu/qemu_domain.h (qemuDomainObjEnterMonitorAsync): New
prototype.
* src/qemu/qemu_process.h (qemuProcessStartCPUs)
(qemuProcessStopCPUs): Add parameter.
* src/qemu/qemu_migration.h (qemuMigrationToFile): Likewise.
(qemuMigrationWaitForCompletion): Make static.
* src/qemu/qemu_domain.c (qemuDomainObjEnterMonitorInternal): Add
parameter.
(qemuDomainObjEnterMonitorAsync): New function.
(qemuDomainObjEnterMonitor, qemuDomainObjEnterMonitorWithDriver):
Update callers.
* src/qemu/qemu_driver.c (qemuDomainSaveInternal)
(qemudDomainCoreDump, doCoreDump, processWatchdogEvent)
(qemudDomainSuspend, qemudDomainResume, qemuDomainSaveImageStartVM)
(qemuDomainSnapshotCreateActive, qemuDomainRevertToSnapshot):
Likewise.
* src/qemu/qemu_process.c (qemuProcessStopCPUs)
(qemuProcessFakeReboot, qemuProcessRecoverMigration)
(qemuProcessRecoverJob, qemuProcessStart): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationToFile)
(qemuMigrationWaitForCompletion, qemuMigrationUpdateJobStatus)
(qemuMigrationJobStart, qemuDomainMigrateGraphicsRelocate)
(doNativeMigrate, doTunnelMigrate, qemuMigrationPerformJob)
(qemuMigrationPerformPhase, qemuMigrationFinish)
(qemuMigrationConfirm): Likewise.
---
My initial smoke testing shows that this fixes 'virsh managedsave',
but I still have more testing to do before I'm convinced I got
everything (for example, I need to test migration still).
src/qemu/THREADS.txt |8 --
src/qemu/qemu_domain.c| 43 +++---
src/qemu/qemu_domain.h|4 +++
src/qemu/qemu_driver.c| 39 +--
src/qemu/qemu_migration.c | 55
src/qemu/qemu_migration.h |5 +--
src/qemu/qemu_process.c | 32 ++
src/qemu/qemu_process.h |7 -
8 files changed, 133 insertions(+), 60 deletions(-)
diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt
index e73076c..125bd5d 100644
--- a/src/qemu/THREADS.txt
+++ b/src/qemu/THREADS.txt
@@ -374,7 +374,7 @@ Design patterns
qemuDriverUnlock(driver);
- * Running asynchronous job
+ * Running asynchronous job with driver lock held
virDomainObjPtr obj;
qemuDomainObjPrivatePtr priv;
@@ -387,7 +387,8 @@ Design patterns
...do prep work...
- if (qemuDomainObjEnterMonitorWithDriver(driver, obj) 0) {
+ if (qemuDomainObjEnterMonitorAsync(driver, obj,
+QEMU_ASYNC_JOB_TYPE) 0) {
/* domain died in the meantime */
goto error;
}
@@ -395,7 +396,8 @@ Design patterns
qemuDomainObjExitMonitorWithDriver(driver, obj);
while (!finished) {
- if (qemuDomainObjEnterMonitorWithDriver(driver, obj) 0) {
+ if (qemuDomainObjEnterMonitorAsync(driver, true, obj,
+QEMU_ASYNC_JOB_TYPE) 0) {
/* domain died in the meantime */
goto error;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2eaaf3a..4cf6888 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -863,14 +863,20 @@ qemuDomainObjEndAsyncJob(struct qemud_driver *driver,
virDomainObjPtr obj)
return virDomainObjUnref(obj);
}
-static int ATTRIBUTE_NONNULL(1)
+static int
qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
bool driver_locked,
- virDomainObjPtr obj)
+ virDomainObjPtr obj,
+ enum qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivatePtr priv = obj-privateData;
-if (priv-job.active == QEMU_JOB_NONE priv-job.asyncJob) {
+if (asyncJob != QEMU_ASYNC_JOB_NONE) {
+if (asyncJob != priv-job.asyncJob) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(unepxected async job %d), asyncJob);
+return -1;
+}
if (qemuDomainObjBeginJobInternal(driver, driver_locked, obj,
QEMU_JOB_ASYNC_NESTED,
QEMU_ASYNC_JOB_NONE) 0)
@@ -930,15 +936,15 @@
