Re: [libvirt] [PATCH] qemu: qxl devices don't support multifunction yet
On 09/27/2011 05:13 AM, Daniel P. Berrange wrote: On Tue, Sep 27, 2011 at 01:19:20AM -0400, Laine Stump wrote: On 09/19/2011 01:32 PM, Daniel P. Berrange wrote: On Mon, Sep 19, 2011 at 07:16:22PM +0200, Marc-André Lureau wrote: Hi hi On Fri, Sep 16, 2011 at 1:38 PM, Marc-André Lureaumlur...@redhat.com wrote: How do we allow other devices to share the slot? It seems to me that qemuDomainPCIAddressSetNextAddr() only allocate whole slot, while making sure there is no conflicts on the same slot. So, if the user wants to use multi function pci device, he should specify the pci address. So adding a check such as: if (!multiFunc info-addr.pci.function != 0) return error(The %s device doesn't support multifunction address) Wen, does that sound reasonable to you? Daniel, did you had time to verify that PCI allocation is per-slot? (It would be nice to get this workaround for the next release) IMHO this kind of hack doesn't belong in libvirt. It is fine for distro vendors to consider as a one off quick-hack for their packages of libvirt, if they don't have time to fix the real QXL bug, but not for libvirt upstream releases. QXL/QEMU should really be fixed since that's where the problem appears to lie. As it stands, Fedora 16 (currently using unpatched libvirt-0.9.6) will be going into beta with QXL video broken for Windows guests, so we need some kind of Fedora-only patch very soon (see the schedule here: https://fedoraproject.org/wiki/Releases/16/Schedule - fortunately just delayed another week) The original patch in this thread: https://www.redhat.com/archives/libvir-list/2011-September/msg00534.html of course doesn't include the above mentioned additional code, and there isn't a followup patch. It would be very good to push a patch to the F16 git for this so it would hopefully get into the beta, but want to make sure what I push is the right thing, so a final patch (and some testing by people with F16 hosts) would be very helpful! When we originally enabled multifunction for all PCI devices, we did so in the belief that this was effectively a no-op for guest OSes. ie it should not have changed guest OS behaviour at all, unless multiple devices were actually inserted in the slot. Evidentally this turns out to be incorrect. In other words we introduced a guest ABI change. This is very bad, because our stated goal is to have a stable guest ABI across libvirt QEMU releases. Thus IMHO we need to disable *all* setting of the 'multifunction=on' parameter for all guests by default, to unbreak the ABI compatibility. Then we should introduce a new parameter 'multifunction='on' in the address type=pci element to allow it to be optionally enabled per device. I just sent a patch to do this, in a separate thread so it doesn't get buried: https://www.redhat.com/archives/libvir-list/2011-September/msg01291.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume
(2011/09/30 14:26), MATSUDA, Daiki wrote: I tried the new snapshot function implemented by Eric Blake. It works very well for QCOW2 disk image system. But I often use LVM2 volume for QEMU virtual machines and tried to take disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only). So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2 volume and create QCOW2 snapshot image. In addition, domain's configuration file is replaced to use snapshot disk image instead of LVM2 volume. configuration file from disk type='block' device='disk driver name='qemu' type='raw' cache='none'/ source dev='dev/VG1/LVM2_dom'/ to disk type='block' device='disk driver name='qemu' type='qcow2' cache='none'/ source dev='dev/VG1/LVM2_dom.1317357844'/ After then, the domain runs well till it is shutdowned. I started the domain, but it does not with following error virtsh # start LVM2_dom error: Failed to start domain LVM2_dom error: 内部エラー Process exited while reading console log output: char Sorry, upper is error: internal error Process exited while reading console log output: char device redirected to /dev/pts/7 qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid argument. I think that if the volume but qcow2 is given libvirt should be refuse, e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver type. But currently the structures concerning with snapshot or disk has no member to hold such a volume driver information. In addition, as we want to add the LVM2 and other volume snapshot function, we hope you add its information and fix. Regards MATSUDA Daiki -- 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
Re: [libvirt] [PATCH] qemu: Check for outstanding async job too
On 29.09.2011 17:26, Eric Blake wrote: On 09/29/2011 09:01 AM, Michal Privoznik wrote: Currently, qemuDomainGetXMLDesc and qemudDomainGetInfo check for outstanding synchronous job before (eventual) monitor entering. However, there can be already async job set, e.g. migration. --- src/qemu/qemu_domain.c | 12 +--- src/qemu/qemu_domain.h |2 ++ src/qemu/qemu_driver.c |4 ++-- 3 files changed, 13 insertions(+), 5 deletions(-) Looks reasonable. ACK. Thanks, pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] conf: remove unused VIR_ENUM_DECL
On Fri, Sep 30, 2011 at 01:40:45AM -0400, Laine Stump wrote: While adding a new enum, I noticed a VIR_ENUM_DECL for a type that doesn't exist. There is also of course no matching VIR_ENUM_IMPL for it. --- src/conf/domain_conf.h |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0bc0042..4a99ea4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1830,7 +1830,6 @@ VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) -VIR_ENUM_DECL(virDomainDeviceAddressMode) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: make PCI multifunction support more manual
On Fri, Sep 30, 2011 at 01:40:46AM -0400, Laine Stump wrote:
When support for was added for PCI multifunction cards (in commit
9f8baf, first included in libvirt 0.9.3), it was done by always
turning on the multifunction bit for all PCI devices. Since that time
it has been realized that this is not an ideal solution, and that the
multifunction bit must be selectively turned on. For example, see
https://bugzilla.redhat.com/show_bug.cgi?id=728174
and the discussion before and after
https://www.redhat.com/archives/libvir-list/2011-September/msg01036.html
This patch modifies multifunction support so that the multifunction=on
option is only added to the qemu commandline for a device if its PCI
address definition has the attribute multifunction='on', e.g.:
address type='pci' domain='0x' bus='0x00'
slot='0x04' function='0x0' multifunction='on'/
[...]
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4972fac..cffaac2 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2118,6 +2118,14 @@
attribute name=function
ref name=pciFunc/
/attribute
+optional
+ attribute name=multifunction
+choice
+ valueon/value
+ valueoff/value
+/choice
+ /attribute
+/optional
/define
define name=driveaddress
optional
okay
[...]
+VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti,
+ VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST,
+ default,
+ on,
+ off)
+
VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST,
block,
file,
@@ -1652,6 +1658,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
info-addr.pci.bus,
info-addr.pci.slot,
info-addr.pci.function);
[...]
+if (multi
+((addr-multi = virDomainDeviceAddressPciMultiTypeFromString(multi))
0)) {
+virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unknown value '%s' for address
'multifunction' attribute),
+ multi);
+goto cleanup;
+
+}
This code allows mutifunction=default input
if you make the test = 0 then it should reject default as
expected...
if (!virDomainDevicePCIAddressIsValid(addr)) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Insufficient specification for PCI
address));
[...]
@@ -1391,11 +1397,11 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
virBufferAsprintf(buf, ,bus=pci.0);
else
virBufferAsprintf(buf, ,bus=pci);
-if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION))
-virBufferAsprintf(buf, ,multifunction=on,addr=0x%x.0x%x,
- info-addr.pci.slot, info-addr.pci.function);
-else
-virBufferAsprintf(buf, ,addr=0x%x, info-addr.pci.slot);
+if (info-addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON)
+virBufferAddLit(buf, ,multifunction=on);
Hum seems to me that if the users explicitely specified mutifunction=off
then that ought to be saved, and hence
else if (info-addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF)
virBufferAddLit(buf, ,multifunction=off);
need to be added (since it's not the default which is 0 that won't
pollute XML where it's not specified).
+virBufferAsprintf(buf, ,addr=0x%x, info-addr.pci.slot);
+if (info-addr.pci.function != 0)
+ virBufferAsprintf(buf, .0x%x, info-addr.pci.function);
} else if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
virBufferAsprintf(buf, ,bus=);
qemuUsbId(buf, info-addr.usb.bus);
ACK with those 2 problems fixed
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: implement getparent for esx
On Thu, Sep 29, 2011 at 02:38:24PM -0600, Eric Blake wrote: Pretty easy to paste together compared to existing functions. * src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function. --- I can only compile-test this; I'm relying on someone with an actual esx setup to actually test it. Also, I didn't see anything in existing code that would efficiently implement virDomainSnapshotNumChildren; there may an API that I'm not aware of, but someone else will have to implement that API (Matthias?) The 2 patches looks just fine to me but agreed someone with the setup should check those and give the final ACK :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Improve the daemon man page
Add informations about the handling of USR2 and fatal signals and document the default log location. diff --git a/daemon/libvirtd.pod.in b/daemon/libvirtd.pod.in index 6e699b8..269e81f 100644 --- a/daemon/libvirtd.pod.in +++ b/daemon/libvirtd.pod.in @@ -69,6 +69,10 @@ Display version information then exit. =head1 SIGNALS On receipt of BSIGHUP libvirtd will reload its configuration. +On receipt of BSIGUSR2 or other fatal signals like BSIGFPE, +BSIGSEGV, BSIGILL, BSIGABRT or BSIGBUS, libvirtd will dump +its internal debug message buffer content to the default log output, +usually B@localstatedir@/log/libvirt/libvirtd.log =head1 FILES @@ -105,6 +109,11 @@ The TLS BServer private key libvirtd will use. The PID file to use, unless overridden by the B-p|B--pid-file option. +=item F@localstatedir@/log/libvirt/libvirtd.log + +The default file where libvirtd will save its logs, this can be overriden +from the B@sysconfdir@/libvirtd.conf configuration file. + =back =head1 EXAMPLES -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] snapshot: implement getparent for esx
2011/9/30 Daniel Veillard veill...@redhat.com: On Thu, Sep 29, 2011 at 02:38:24PM -0600, Eric Blake wrote: Pretty easy to paste together compared to existing functions. * src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function. --- I can only compile-test this; I'm relying on someone with an actual esx setup to actually test it. Also, I didn't see anything in existing code that would efficiently implement virDomainSnapshotNumChildren; there may an API that I'm not aware of, but someone else will have to implement that API (Matthias?) The 2 patches looks just fine to me but agreed someone with the setup should check those and give the final ACK :-) Daniel I'll probably have some time this weekend to have a look at it. -- 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 1/1] [RFC] Parallels Server Bare Metal driver stub
On Wednesday, September 28, 2011 06:10:19 PM Daniel P. Berrange wrote: Any pointers ? All I found was http://www.parallels.com/ptn/download/sdk/ and it's quite silent on code availability and Licence for the libraries. It has a proprietary license and not open sourced now. Is it a problem? If the license is not LGPLv2+ compatible, then it can't be used by libvirt, regardless of whether it is directly linked, or dlopened. In other words using 'dlopen' doesn't magically solve the license compatibility problem. Will we solve the issue if libvirt will be statically linked with SDK library (as Daniel requests) and SDK library itself will be distributed in binary form under BSD license conditions (which is LGPLv2 compatible)? -- Thanks, Dmitry. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] [RFC] Parallels Server Bare Metal driver stub
libvirt will be statically linked with SDK library Sorry, I meant dynamically but at compilation stage instead of dlopen() On Friday, September 30, 2011 12:21:05 PM Dmitry Mishin wrote: On Wednesday, September 28, 2011 06:10:19 PM Daniel P. Berrange wrote: Any pointers ? All I found was http://www.parallels.com/ptn/download/sdk/ and it's quite silent on code availability and Licence for the libraries. It has a proprietary license and not open sourced now. Is it a problem? If the license is not LGPLv2+ compatible, then it can't be used by libvirt, regardless of whether it is directly linked, or dlopened. In other words using 'dlopen' doesn't magically solve the license compatibility problem. Will we solve the issue if libvirt will be statically linked with SDK library (as Daniel requests) and SDK library itself will be distributed in binary form under BSD license conditions (which is LGPLv2 compatible)? -- Thanks, Dmitry. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] [RFC] Parallels Server Bare Metal driver stub
On Fri, Sep 30, 2011 at 12:21:05PM +0400, Dmitry Mishin wrote: On Wednesday, September 28, 2011 06:10:19 PM Daniel P. Berrange wrote: Any pointers ? All I found was http://www.parallels.com/ptn/download/sdk/ and it's quite silent on code availability and Licence for the libraries. It has a proprietary license and not open sourced now. Is it a problem? If the license is not LGPLv2+ compatible, then it can't be used by libvirt, regardless of whether it is directly linked, or dlopened. In other words using 'dlopen' doesn't magically solve the license compatibility problem. Will we solve the issue if libvirt will be statically linked with SDK library (as Daniel requests) and SDK library itself will be distributed in binary form under BSD license conditions (which is LGPLv2 compatible)? Yes, linking to a BSD licensed library is fine from a licensing point of view. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Adding new filesystem 'proxy' to 9p
On Thu, Sep 29, 2011 at 11:42:47PM +0530, M. Mohan Kumar wrote: On Wednesday, September 28, 2011 08:29:06 PM Daniel P. Berrange wrote: On Wed, Sep 28, 2011 at 07:49:34PM +0530, M. Mohan Kumar wrote: Pass-through security model in QEMU 9p server needs root privilege to do few file operations (like chown, chmod to any mode/uid:gid). There are two issues in pass-through security model 1) TOCTTOU vulnerability: Following symbolic links in the server could provide access to files beyond 9p export path. 2) When libvirt is configured to run qemu as non-root user (for example, if qemu is configured to run as normal user 'qemu'), running file operations on pass-through security model would fail because it needs root privileges. To overcome above issues, following approach is suggested: A new filesytem type 'proxy' is introduced. Proxy FS uses chroot + socket combination for securing the vulnerability known with following symbolic links. Intention of adding a new filesystem type is to allow qemu to run in non-root mode, but doing privileged operations using socket IO. A new binary (known as proxy helper) will be provided as part of qemu. Proxy helper will chroot into 9p export path and create a socket pair or a named socket based on the command line parameter. Qemu and proxy helper will communicate using this socket. We need following changes in the libvirt code to accomodate new 'proxy' filesystem type: If qemu 9p server is configured to use 'proxy' FS, libvirt will do * Create a socket pair * invoke proxy_helper binary with one of the socket id from the pair as command line parameters to it with root privilege * invoke qemu with one of socket id from the pair as paramter to qemu virtfs after dropping to the configured user privilege. ie, libvirt will invoke proxy_helper as: proxy_helper -i socket_fd_from_socket_pair -p 9p-path-to-export and qemu will be invoked with following virtfs parameter: -virtfs proxy,id=id,sock_fd=socket_fd_from_socket_pair ,path=/tmp/,security_model=prox,mount_tag=v_pass Thank you Daniel for the quick response, I was on leave and I could not respond to you immediately. Interesting proposal. Explicitly comparing the security characteristics of running QEMU as root, vs using the proxy helper * QEMU run as root - QEMU is root, with full capabilities - QEMU has read/write any file/dir, regardless of whether it is exported via 9p * QEMU run as non-root, with proxy_helper root - QEMU is non-root, with no capabilities - QEMU has write to only files with matching UID/GID - proxy_helper is root, with full capabilities - proxy_helper has read/write to any file/dir Since QEMU can send arbitrary FS calls to the proxy_helper, the overall security of the system clearly depends on the security of the proxy_helper process. QEMU can't send arbitrary FS calls to the proxy helper. All interfaces are defined and if qemu sends arbitrary FS command, proxy helper would return an EIO. For example T_OPEN is one of the supported interface between 9p qemu server and proxy helper. For more information please look at my chroot patchset that I posted to qemu-devel few weeks ago. http://lists.gnu.org/archive/html/qemu-devel/2011-09/msg00671.html Ok, perhaps I didn't explain what I meant by 'arbitrary FS calls' very well here. Lets, say the communication protocol between QEMU and the proxy helper has a set of FS calls to cover 'read', 'write', 'open', 'unlink', 'link'. And we have a 9pfs directory to give to the guest '/export/to-the-guest'. In normal use, the QEMU process can send things like open(/export/to-the-guest/somefile). A malicious QEMU can send open(/etc/shadow) Or any other path that it cares about, whether or not, the path is within the location exported to the 9pfs server. The security of the overall system, thus relies on the proxy helper enforcing that the arbitrary FS calls from QEMU, are in fact only within '/export/to-the-guest' and not other directories. If we assume that QEMU gets exploited, and that QEMU can find some flaw in the proxy_helper that it can exploit, what damage can the proxy_helper do ? Clearly we want it to not be able to read/write any files other than those exported, even when it becomes compromised, ideally also without requiring SELinux/AppArmour to make it safe. If proxy_helper is running as root with full capabilities, it can trivially escape the chroot[1], so this isn't all that nice for overall system security. When proxy helper process is forked, its made sure that all open file descriptor are closed except the socket descriptor, so that proxy helper can't escape chroot jail. Unfortunately that is not sufficient. Even if you have closed all file descriptors except the socket, you can still
[libvirt] [RFC PATCH 0/2] Add some systemtap probes to RPC code
Trying to investigate debug the keepalive series I decided we
needed a better way to trace the RPC protocol than the regular
debug messages. Ideally we'd have a wireshark dissector but that
seems like it will be an awful lot of work todo. So instead I
have added a few systemtap probes to the RPC call which fire
everytime an RPC message is queued for dispatch, or a completed
RPC message is read off the wire
Now I can see all communications using a script like this
# cat watch.stp EOF
function msginfo(prefix, prog, version, proc, type, status, serial)
{
if (type == 0)
typestr = call
else if (type == 1)
typestr = reply
else if (type == 2)
typestr = message
else if (type == 3)
typestr = stream
else
typestr = unknown
if (status == 0)
statusstr = ok
else if (status == 1)
statusstr = error
else if (status == 2)
statusstr = continue
else
statusstr = unknown
printf(%s program: %-10d version: %d procedure: %-4d type: %u:%-9s status:
%u:%-9s serial: %-4d\n,
prefix, prog, version, proc, type, typestr, status, statusstr, serial);
}
probe libvirt.rpc.server_msg_rx {
msginfo(S, prog, vers, proc, type, status, serial)
}
probe libvirt.rpc.server_msg_tx {
msginfo(S, prog, vers, proc, type, status, serial)
}
probe libvirt.rpc.client_msg_rx {
msginfo(C, prog, vers, proc, type, status, serial)
}
probe libvirt.rpc.client_msg_tx {
msginfo(C, prog, vers, proc, type, status, serial)
}
EOF
# stap watch.stp
C program: 536903814 version: 1 procedure: 66 type: 0:call status:
0:okserial: 0
S program: 536903814 version: 1 procedure: 66 type: 0:call status:
0:okserial: 0
S program: 536903814 version: 1 procedure: 66 type: 1:reply status:
0:okserial: 0
C program: 536903814 version: 1 procedure: 66 type: 1:reply status:
0:okserial: 0
C program: 536903814 version: 1 procedure: 1type: 0:call status:
0:okserial: 1
S program: 536903814 version: 1 procedure: 1type: 0:call status:
0:okserial: 1
S program: 536903814 version: 1 procedure: 1type: 1:reply status:
0:okserial: 1
C program: 536903814 version: 1 procedure: 1type: 1:reply status:
0:okserial: 1
C program: 536903814 version: 1 procedure: 110 type: 0:call status:
0:okserial: 2
S program: 536903814 version: 1 procedure: 110 type: 0:call status:
0:okserial: 2
S program: 536903814 version: 1 procedure: 110 type: 1:reply status:
0:okserial: 2
C program: 536903814 version: 1 procedure: 110 type: 1:reply status:
0:okserial: 2
C program: 536903814 version: 1 procedure: 23 type: 0:call status:
0:okserial: 3
S program: 536903814 version: 1 procedure: 23 type: 0:call status:
0:okserial: 3
S program: 536903814 version: 1 procedure: 23 type: 1:reply status:
0:okserial: 3
C program: 536903814 version: 1 procedure: 23 type: 1:reply status:
0:okserial: 3
C program: 536903814 version: 1 procedure: 19 type: 0:call status:
0:okserial: 4
S program: 536903814 version: 1 procedure: 19 type: 0:call status:
0:okserial: 4
S program: 536903814 version: 1 procedure: 19 type: 1:reply status:
0:okserial: 4
C program: 536903814 version: 1 procedure: 19 type: 1:reply status:
0:okserial: 4
C program: 536903814 version: 1 procedure: 16 type: 0:call status:
0:okserial: 5
S program: 536903814 version: 1 procedure: 16 type: 0:call status:
0:okserial: 5
S program: 536903814 version: 1 procedure: 16 type: 1:reply status:
0:okserial: 5
C program: 536903814 version: 1 procedure: 16 type: 1:reply status:
0:okserial: 5
C program: 536903814 version: 1 procedure: 151 type: 0:call status:
0:okserial: 6
S program: 536903814 version: 1 procedure: 151 type: 0:call status:
0:okserial: 6
S program: 536903814 version: 1 procedure: 151 type: 1:reply status:
0:okserial: 6
C program: 536903814 version: 1 procedure: 151 type: 1:reply status:
0:okserial: 6
C program: 536903814 version: 1 procedure: 15 type: 0:call status:
0:okserial: 7
S program: 536903814 version: 1 procedure: 15 type: 0:call status:
0:okserial: 7
S program: 536903814 version: 1 procedure: 15 type: 1:reply status:
0:okserial: 7
C program: 536903814 version: 1 procedure: 15 type: 1:reply status:
0:okserial: 7
C program: 536903814 version: 1 procedure: 183 type: 0:call status:
0:okserial: 8
S program: 536903814 version: 1 procedure: 183 type: 0:call status:
0:okserial: 8
S program: 536903814 version: 1 procedure: 183 type: 1:reply
[libvirt] [PATCH 2/2] Add some DTrace/SystemTAP probes to the RPC code
From: Daniel P. Berrange berra...@redhat.com --- daemon/libvirtd.h| 28 +--- src/Makefile.am | 28 +++-- src/internal.h | 70 ++ src/libvirt.stp | 35 + src/probes.d |7 src/rpc/virnetclient.c | 13 +-- src/rpc/virnetserverclient.c |9 + 7 files changed, 157 insertions(+), 33 deletions(-) create mode 100644 src/libvirt.stp create mode 100644 src/probes.d diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 6d6460e..43f5921 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -45,32 +45,8 @@ # include probes.h # endif /* LIBVIRTD_PROBES_H */ -/* Systemtap 1.2 headers have a bug where they cannot handle a - * variable declared with array type. Work around this by casting all - * arguments. This is some gross use of the preprocessor because - * PROBE is a var-arg macro, but it is better than the alternative of - * making all callers to PROBE have to be aware of the issues. And - * hopefully, if we ever add a call to PROBE with other than 2 or 3 - * end arguments, you can figure out the pattern to extend this hack. - */ -# define VIR_COUNT_ARGS(...) VIR_ARG5(__VA_ARGS__, 4, 3, 2, 1) -# define VIR_ARG5(_1, _2, _3, _4, _5, ...) _5 -# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) -# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__) - -/* The double cast is necessary to silence gcc warnings; any pointer - * can safely go to intptr_t and back to void *, which collapses - * arrays into pointers; while any integer can be widened to intptr_t - * then cast to void *. */ -# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a)) -# define VIR_ADD_CAST2(a, b) \ -VIR_ADD_CAST(a), VIR_ADD_CAST(b) -# define VIR_ADD_CAST3(a, b, c)\ -VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c) - -# define VIR_ADD_CASTS(...)\ -VIR_ADD_CAST_EXPAND(VIR_ADD_CAST, VIR_COUNT_ARGS(__VA_ARGS__), \ -__VA_ARGS__) +# undef PROBE_EXPAND +# undef PROBE # define PROBE_EXPAND(NAME, ARGS) NAME(ARGS) # define PROBE(NAME, FMT, ...) \ diff --git a/src/Makefile.am b/src/Makefile.am index 7281802..d974904 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -25,6 +25,9 @@ AM_LDFLAGS = $(COVERAGE_LDFLAGS) EXTRA_DIST = $(conf_DATA) util/keymaps.csv BUILT_SOURCES = +CLEANFILES = +DISTCLEANFILES = +MAINTAINERCLEANFILES = if WITH_NETWORK UUID=$(shell uuidgen 2/dev/null) @@ -1248,6 +1251,25 @@ libvirt_la_CFLAGS = -DIN_LIBVIRT $(AM_CFLAGS) # picked out for us. libvirt_la_DEPENDENCIES = $(libvirt_la_BUILT_LIBADD) $(LIBVIRT_SYMBOL_FILE) +if WITH_DTRACE +libvirt_la_LIBADD += probes.o +nodist_libvirt_la_SOURCES = probes.h + +BUILT_SOURCES += probes.h + +tapsetdir = $(datadir)/systemtap/tapset +tapset_DATA = libvirt.stp + +probes.h: probes.d + $(AM_V_GEN)$(DTRACE) -o $@ -h -s $ + +probes.o: probes.d + $(AM_V_GEN)$(DTRACE) -o $@ -G -s $ + +CLEANFILES += probes.h probes.o +endif + + # Create an automake convenience library version of libvirt_la, # just for testing, since the test harness requires access to internal # bits and pieces that we don't want to make publicly accessible. @@ -1549,6 +1571,6 @@ if WITH_NETWORK endif rmdir $(DESTDIR)$(localstatedir)/lib/libvirt ||: -CLEANFILES = *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.i *.s -DISTCLEANFILES = $(GENERATED_SYM_FILES) -MAINTAINERCLEANFILES = $(REMOTE_DRIVER_GENERATED) $(VIR_NET_RPC_GENERATED) $(ESX_DRIVER_GENERATED) +CLEANFILES += *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.i *.s +DISTCLEANFILES += $(GENERATED_SYM_FILES) +MAINTAINERCLEANFILES += $(REMOTE_DRIVER_GENERATED) $(VIR_NET_RPC_GENERATED) $(ESX_DRIVER_GENERATED) diff --git a/src/internal.h b/src/internal.h index 1997031..18867fd 100644 --- a/src/internal.h +++ b/src/internal.h @@ -242,4 +242,74 @@ /* divide value by size, rounding up */ # define VIR_DIV_UP(value, size) (((value) + (size) - 1) / (size)) + +# if WITH_DTRACE +# ifndef LIBVIRT_PROBES_H +# define LIBVIRT_PROBES_H +# include probes.h +# endif /* LIBVIRT_PROBES_H */ + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG9(__VA_ARGS__, 8, 7, 6, 5, 4, 3, 2, 1) +# define VIR_ARG9(_1, _2, _3, _4, _5, _6, _7, _8, _9, ...) _9 +# define VIR_ADD_CAST_EXPAND(a, b, ...)
[libvirt] [PATCH 1/2] Make libvirt.so include the RPC server code
From: Daniel P. Berrange berra...@redhat.com To avoid static linking libvirtd to the RPC server code, which then prevents sane introduction of DTrace probes, put it all in the libvirt.so, and export it * daemon/Makefile.am: Don't link to RPC libraries * src/Makefile.am: Link all RPC libraries to libvirt.so * src/libvirt_private.syms: Export all RPC functions --- daemon/Makefile.am |2 - src/Makefile.am |2 +- src/libvirt_private.syms | 76 ++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 690bf85..046bff3 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -110,8 +110,6 @@ libvirtd_LDADD =\ $(POLKIT_LIBS) libvirtd_LDADD += \ - ../src/libvirt-net-rpc-server.la \ - ../src/libvirt-net-rpc.la \ ../src/libvirt-qemu.la if ! WITH_DRIVER_MODULES diff --git a/src/Makefile.am b/src/Makefile.am index d983d28..7281802 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -612,7 +612,7 @@ libvirt_driver_remote_la_CFLAGS = \ -I@top_srcdir@/src/rpc \ $(AM_CFLAGS) libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) libvirt-net-rpc-client.la libvirt-net-rpc.la +libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) libvirt-net-rpc-client.la libvirt-net-rpc-server.la libvirt-net-rpc.la if WITH_DRIVER_MODULES libvirt_driver_remote_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_remote_la_LDFLAGS += -module -avoid-version diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2a3fab..e086253 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1162,6 +1162,82 @@ virFileDirectFdNew; virFileFclose; virFileFdopen; +# rpc +virNetMessageClear; +virNetMessageEncodeHeader; +virNetMessageEncodePayload; +virNetMessageFree; +virNetMessageNew; +virNetMessageQueuePush; +virNetMessageQueueServe; +virNetMessageSaveError; +virNetSASLContextCheckIdentity; +virNetSASLContextNewServer; +virNetSASLSessionExtKeySize; +virNetSASLSessionFree; +virNetSASLSessionGetIdentity; +virNetSASLSessionGetKeySize; +virNetSASLSessionListMechanisms; +virNetSASLSessionNewServer; +virNetSASLSessionSecProps; +virNetSASLSessionServerStart; +virNetSASLSessionServerStep; +virNetServerAddProgram; +virNetServerAddService; +virNetServerAddSignalHandler; +virNetServerAutoShutdown; +virNetServerClientAddFilter; +virNetServerClientClose; +virNetServerClientDelayedClose; +virNetServerClientFree; +virNetServerClientGetAuth; +virNetServerClientGetFD; +virNetServerClientGetLocalIdentity; +virNetServerClientGetPrivateData; +virNetServerClientGetReadonly; +virNetServerClientGetTLSKeySize; +virNetServerClientHasTLSSession; +virNetServerClientImmediateClose; +virNetServerClientIsSecure; +virNetServerClientLocalAddrString; +virNetServerClientRef; +virNetServerClientRemoteAddrString; +virNetServerClientRemoveFilter; +virNetServerClientSendMessage; +virNetServerClientSetCloseHook; +virNetServerClientSetIdentity; +virNetServerClientSetPrivateData; +virNetServerClientSetSASLSession; +virNetServerClose; +virNetServerFree; +virNetServerIsPrivileged; +virNetServerNew; +virNetServerProgramFree; +virNetServerProgramGetID; +virNetServerProgramGetVersion; +virNetServerProgramMatches; +virNetServerProgramNew; +virNetServerProgramRef; +virNetServerProgramSendReplyError; +virNetServerProgramSendStreamData; +virNetServerProgramSendStreamError; +virNetServerQuit; +virNetServerRef; +virNetServerRun; +virNetServerServiceFree; +virNetServerServiceNewTCP; +virNetServerServiceNewUNIX; +virNetServerUpdateServices; +virNetSocketDupFD; +virNetSocketFree; +virNetSocketGetFD; +virNetSocketListen; +virNetSocketNewConnectTCP; +virNetSocketNewListenUNIX; +virNetTLSContextFree; +virNetTLSContextNewServer; +virNetTLSContextNewServerPath; + # virpidfile.h virPidFileAcquire; -- 1.7.6.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Add missing package and c:include in gir
This is required for generating source-level bindings, for vala --- libvirt-gconfig/Makefile.am |2 ++ libvirt-glib/Makefile.am|2 ++ libvirt-gobject/Makefile.am |2 ++ 3 files changed, 6 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index 6f1c628..180d5ee 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -72,6 +72,8 @@ LibvirtGConfig-1.0.gir: libvirt-gconfig-1.0.la $(G_IR_SCANNER) Makefile.am -I$(top_builddir) \ --verbose \ --pkg=gobject-2.0 \ +--c-include=libvirt-gconfig/libvirt-gconfig.h \ +--pkg-export=libvirt-gconfig-1.0 \ $(srcdir)/libvirt-gconfig.h \ $(GCONFIG_SOURCE_FILES:%=$(srcdir)/%) \ $(GCONFIG_HEADER_FILES:%=$(srcdir)/%) diff --git a/libvirt-glib/Makefile.am b/libvirt-glib/Makefile.am index 4da0a84..0638a64 100644 --- a/libvirt-glib/Makefile.am +++ b/libvirt-glib/Makefile.am @@ -53,6 +53,8 @@ LibvirtGLib-1.0.gir: libvirt-glib-1.0.la $(G_IR_SCANNER) Makefile.am -I$(srcdir) \ --pkg=glib-2.0 \ --pkg=gthread-2.0 \ +--c-include=libvirt-glib/libvirt-glib.h \ +--pkg-export=libvirt-glib-1.0 \ $(libvirt_glib_1_0_la_SOURCES:%=$(srcdir)/%) girdir = $(datadir)/gir-1.0 diff --git a/libvirt-gobject/Makefile.am b/libvirt-gobject/Makefile.am index a50878f..9430d40 100644 --- a/libvirt-gobject/Makefile.am +++ b/libvirt-gobject/Makefile.am @@ -117,6 +117,8 @@ LibvirtGObject-1.0.gir: libvirt-gobject-1.0.la $(G_IR_SCANNER) Makefile.am --verbose \ --pkg=gobject-2.0 \ --pkg=gthread-2.0 \ +--c-include=libvirt-gobject/libvirt-gobject.h \ +--pkg-export=libvirt-gobject-1.0 \ $(srcdir)/libvirt-gobject.h \ $(GOBJECT_SOURCE_FILES:%=$(srcdir)/%) \ $(GOBJECT_HEADER_FILES:%=$(srcdir)/%) \ -- 1.7.6.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Openstack] [RFC PATCH] lxc: don't return error on GetInfo when cgroups not yet set up
Quoting Serge E. Hallyn (serge.hal...@canonical.com): Quoting Daniel P. Berrange (berra...@redhat.com): On Wed, Sep 28, 2011 at 02:14:52PM -0500, Serge E. Hallyn wrote: Nova (openstack) calls libvirt to create a container, then periodically checks using GetInfo to see whether the container is up. If it does this too quickly, then libvirt returns an error, which in libvirt.py causes an exception to be raised, the same type as if the container was bad. lxcDomainGetInfo(), holds a mutex on 'dom' for the duration of its execution. It checks for virDomainObjIsActive() before trying to use the cgroups. Yes, it does, but lxcDomainStart(), holds the mutex on 'dom' for the duration of its execution, and does not return until the container is running and cgroups are present. No. It calls the lxc_controller with --background. The controller main task in turn exits before the cgroups have been set up. There is the race. So what is the right fix here? Should the controller write out another file when it is past the part which should be locked, and the driver waits for that file to exist before it drops the driver mutex? If we do that, do we risk having the driver hang when the controller has hung? -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] lxc: don't return error on GetInfo when cgroups not yet set up
On Thu, Sep 29, 2011 at 10:12:17PM -0500, Serge E. Hallyn wrote: Quoting Daniel P. Berrange (berra...@redhat.com): On Wed, Sep 28, 2011 at 02:14:52PM -0500, Serge E. Hallyn wrote: Nova (openstack) calls libvirt to create a container, then periodically checks using GetInfo to see whether the container is up. If it does this too quickly, then libvirt returns an error, which in libvirt.py causes an exception to be raised, the same type as if the container was bad. lxcDomainGetInfo(), holds a mutex on 'dom' for the duration of its execution. It checks for virDomainObjIsActive() before trying to use the cgroups. Yes, it does, but lxcDomainStart(), holds the mutex on 'dom' for the duration of its execution, and does not return until the container is running and cgroups are present. No. It calls the lxc_controller with --background. The controller main task in turn exits before the cgroups have been set up. There is the race. The lxcDomainStart() method isn't actually waiting on the child pid directly, so the --background flag ought not to matter. We have a pipe that we pass into the controller, which we wait on for a notification after running the process. The controller does not notify the 'handshake' FD until after cgroups have been setup, unless I'm mis-interpreting our code Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] lxc: don't return error on GetInfo when cgroups not yet set up
Quoting Daniel P. Berrange (berra...@redhat.com): On Thu, Sep 29, 2011 at 10:12:17PM -0500, Serge E. Hallyn wrote: Quoting Daniel P. Berrange (berra...@redhat.com): On Wed, Sep 28, 2011 at 02:14:52PM -0500, Serge E. Hallyn wrote: Nova (openstack) calls libvirt to create a container, then periodically checks using GetInfo to see whether the container is up. If it does this too quickly, then libvirt returns an error, which in libvirt.py causes an exception to be raised, the same type as if the container was bad. lxcDomainGetInfo(), holds a mutex on 'dom' for the duration of its execution. It checks for virDomainObjIsActive() before trying to use the cgroups. Yes, it does, but lxcDomainStart(), holds the mutex on 'dom' for the duration of its execution, and does not return until the container is running and cgroups are present. No. It calls the lxc_controller with --background. The controller main task in turn exits before the cgroups have been set up. There is the race. The lxcDomainStart() method isn't actually waiting on the child pid directly, so the --background flag ought not to matter. We have a pipe that we pass into the controller, which we wait on for a notification after running the process. The controller does not notify the 'handshake' FD until after cgroups have been setup, unless I'm mis-interpreting our code That's the call to lxcContainerWaitForContinue(), right? If so, that's done by lxcContainerChild(), which is called by the lxc_controller. AFAICS there is nothing in the lxc_driver which will wait on that before dropping the driver-lock mutex. -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH] snapshot: better virsh handling of missing current, parent
Previously, virsh 'snapshot-parent' and 'snapshot-current' were
completely silent in the case where the code conclusively proved
there was no parent or current snapshot, but differed in exit
status; this silence caused some confusion on whether the commands
worked. Furthermore, commit d1be48f introduced a regression where
snapshot-parent would leak output about an unknown function, but
only on the first attempt, when talking to an older server that
lacks virDomainSnapshotGetParent. This changes things to consistenly
report an error message and exit with status 1 when no snapshot
exists, and to avoid leaking unknown function warnings when using
fallbacks.
* tools/virsh.c (vshGetSnapshotParent): Alter signature, to
distinguish between real error and missing parent. Don't pollute
last_error on success.
(cmdSnapshotParent): Adjust caller. Always output message on
failure.
(vshGetSnapshotParent): Adjust caller.
(cmdSnapshotCurrent): Always output message on failure.
---
This patch is an RFC because of backwards-compatibility concerns:
Currently, snapshot-current outputs nothing but has status 0 if
there is no current snapshot; but snapshot-parent outputs nothing
but has status 1 if there is no parent snapshot. Either way, we
have an inconsistency that needs to be fixed, and gaining consistency
means breaking backwards compatibility with at least one command.
Approach 1 (this patch): change snapshot-parent and snapshot-current
to always output something, whether to stdout on success or to
stderr on failure, with lack of a snapshot being considered a
failure (where snapshot-current used to treat it as success).
Approach 2 (not written) since snapshot-current existed much longer,
makesnapshot-parent consistent by giving status 0 when it is silent.
Preferences? (I guess mine is approach 1, by evidence of this patch).
tools/virsh.c | 54 +++---
1 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 1909dce..179cda5 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12945,6 +12945,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
char *xml = NULL;
const char *snapshotname = NULL;
unsigned int flags = 0;
+const char *domname;
if (vshCommandOptBool(cmd, security-info))
flags |= VIR_DOMAIN_XML_SECURE;
@@ -12952,7 +12953,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
if (!vshConnectionUsability(ctl, ctl-conn))
goto cleanup;
-dom = vshCommandOptDomain(ctl, cmd, NULL);
+dom = vshCommandOptDomain(ctl, cmd, domname);
if (dom == NULL)
goto cleanup;
@@ -12986,9 +12987,12 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
}
current = virDomainHasCurrentSnapshot(dom, 0);
-if (current 0)
+if (current 0) {
+goto cleanup;
+} else if (!current) {
+vshError(ctl, _(domain '%s' has no current snapshot), domname);
goto cleanup;
-else if (current) {
+} else {
const char *name = NULL;
if (!(snapshot = virDomainSnapshotCurrent(dom, 0)))
@@ -13010,6 +13014,8 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
+if (!ret)
+virshReportError(ctl);
VIR_FREE(xml);
if (snapshot)
virDomainSnapshotFree(snapshot);
@@ -13020,26 +13026,33 @@ cleanup:
}
/* Helper function to get the name of a snapshot's parent. Caller
- * must free the result. */
-static char *
-vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot)
+ * must free the result. Returns 0 on success (including when it was
+ * proven no parent exists), and -1 on failure with error reported
+ * (such as no snapshot support or domain deleted in meantime). */
+static int
+vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot,
+ char **parent_name)
{
virDomainSnapshotPtr parent = NULL;
char *xml = NULL;
xmlDocPtr xmldoc = NULL;
xmlXPathContextPtr ctxt = NULL;
-char *parent_name = NULL;
+int ret = -1;
+
+*parent_name = NULL;
/* Try new API, since it is faster. */
if (!ctl-useSnapshotGetXML) {
parent = virDomainSnapshotGetParent(snapshot, 0);
if (parent) {
-/* API works, and virDomainSnapshotGetName will succeed */
-parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent));
+/* API works, and virDomainSnapshotGetName will be accurate */
+*parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent));
+ret = 0;
goto cleanup;
}
if (last_error-code == VIR_ERR_NO_DOMAIN_SNAPSHOT) {
/* API works, and we found a root with no parent */
+ret = 0;
goto cleanup;
}
/* API didn't work, fall back to XML scraping. */
@@ -13054,15 +13067,23 @@ vshGetSnapshotParent(vshControl *ctl,
[libvirt] [libvirt-glib] RFC: delay event handle freeing to avoid dead lock
Can be reproduced with the updated test.
---
examples/conn-test.c | 20 +++-
libvirt-glib/libvirt-glib-event.c | 17 +
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/examples/conn-test.c b/examples/conn-test.c
index 08045a4..8ea5ad7 100644
--- a/examples/conn-test.c
+++ b/examples/conn-test.c
@@ -31,12 +31,19 @@ do_connection_open(GObject *source,
{
GVirConnection *conn = GVIR_CONNECTION(source);
GError *err = NULL;
-GMainLoop *loop = opaque;
if (!gvir_connection_open_finish(conn, res, err)) {
g_error(%s, err-message);
}
g_print(Connected to libvirt\n);
+g_object_unref(conn);
+}
+
+static void quit(gpointer data,
+ GObject *where_the_object_was)
+{
+GMainLoop *loop = data;
+
g_main_loop_quit(loop);
}
@@ -47,19 +54,22 @@ int main(int argc, char **argv)
gvir_init_object(argc, argv);
-if (argc != 2)
+if (argc != 2) {
g_error(syntax: %s URI, argv[0]);
-
-conn = gvir_connection_new(argv[1]);
+return 1;
+}
loop = g_main_loop_new(g_main_context_default(),
TRUE);
-gvir_connection_open_async(conn, NULL, do_connection_open, loop);
+conn = gvir_connection_new(argv[1]);
+g_object_weak_ref(G_OBJECT(conn), quit, loop);
+
gvir_connection_open_async(conn, NULL, do_connection_open, loop);
g_main_loop_run(loop);
+
return 0;
}
diff --git a/libvirt-glib/libvirt-glib-event.c
b/libvirt-glib/libvirt-glib-event.c
index a785c93..8b1bddf 100644
--- a/libvirt-glib/libvirt-glib-event.c
+++ b/libvirt-glib/libvirt-glib-event.c
@@ -195,6 +195,18 @@ cleanup:
g_mutex_unlock(eventlock);
}
+static gboolean
+_handle_remove(gpointer data)
+{
+struct gvir_event_handle *h = data;
+
+if (h-ff)
+(h-ff)(h-opaque);
+free(h);
+
+return FALSE;
+}
+
static int
gvir_event_handle_remove(int watch)
{
@@ -217,10 +229,7 @@ gvir_event_handle_remove(int watch)
g_source_remove(data-source);
data-source = 0;
data-events = 0;
-if (data-ff)
-(data-ff)(data-opaque);
-free(data);
-
+g_idle_add(_handle_remove, data);
ret = 0;
cleanup:
--
1.7.6.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] qemu: make PCI multifunction support more manual
(V2: address Daniel Veillard's two review points (don't allow multifunction='default', and add multifunction=off to the qemu commandline when that's what the XML says), and modify the checks for duplicate PCI address usage attempts to account for multifunction=off on a device's function 0, per IRC discussion with Dan Berrange (see new qemuCollectPCIAddress()). As a side effect, attempts to re-use the same PCI address are now logged rather than silently generating an error.) When support for was added for PCI multifunction cards (in commit 9f8baf, first included in libvirt 0.9.3), it was done by always turning on the multifunction bit for all PCI devices. Since that time it has been realized that this is not an ideal solution, and that the multifunction bit must be selectively turned on. For example, see https://bugzilla.redhat.com/show_bug.cgi?id=728174 and the discussion before and after https://www.redhat.com/archives/libvir-list/2011-September/msg01036.html This patch modifies multifunction support so that the multifunction=on option is only added to the qemu commandline for a device if its PCI address definition has the attribute multifunction='on', e.g.: address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0' multifunction='on'/ In practice, the multifunction bit should only be turned on if function='0' AND other functions will be used in the same slot - it usually isn't needed for functions 1-7 (although there are apparently some exceptions, e.g. the Intel X53 according to the QEMU source code), and should never be set if only function 0 will be used in the slot. The test cases have been changed accordingly to illustrate. With this patch in place, if a user attempts to assign multiple functions in a slot without setting the multifunction bit for function 0, libvirt will issue an error when the domain is defined, and the define operation will fail. In the future, we may decide to detect this situation and automatically add multifunction=on to avoid the error; even then it will still be useful to have a manual method of turning on multifunction since, as stated above, there are some devices that excpect it to be turned on for all functions in a slot. A side effect of this patch is that attempts to use the same PCI address for two different devices will now log an error (previously this would cause the domain define operation to fail, but there would be no log message generated). Because the function doing this log was almost completely rewritten, I didn't think it worthwhile to make a separate patch for that fix (the entire patch would immediately be obsoleted). --- docs/formatdomain.html.in | 29 +-- docs/schemas/domaincommon.rng |8 ++ src/conf/domain_conf.c | 22 +- src/conf/domain_conf.h | 10 +++ src/libvirt_private.syms |2 + src/qemu/qemu_command.c| 81 .../qemuxml2argv-multifunction-pci-device.args | 18 ++-- .../qemuxml2argv-multifunction-pci-device.xml |6 +- .../qemuxml2argv-usb-ich9-companion.args | 10 +- .../qemuxml2argv-usb-ich9-companion.xml|2 +- .../qemuxml2argv-usb-ich9-ehci-addr.args |2 +- .../qemuxml2argv-usb-piix3-controller.args |2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 10 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml |2 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 20 +++--- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml |4 +- 16 files changed, 165 insertions(+), 63 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 49a2c09..6308ebc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1118,10 +1118,14 @@ The codetype/code attribute is mandatory, and is typically pci or drive. For a pci controller, additional attributes for codebus/code, codeslot/code, -and codefunction/code must be present, as well as an -optional codedomain/code. For a drive controller, -additional attributes codecontroller/code, codebus/code, +and codefunction/code must be present, as well as +optional codedomain/code +and codemultifunction/codespan class=sincesince +0.9.7, requires QEMU 0.13/span (defaults to 'off'). For a +drive controller, additional attributes +codecontroller/code, codebus/code, and codeunit/code are available, each defaulting to 0. + /dd /dl @@ -1298,7 +1302,7 @@ lt;/controllergt; lt;controller type='usb' index='0' model='ich9-uhci1'gt; lt;master startport='0'/gt; - lt;address type='pci' domain='0' bus='0' slot='4' function='0'/gt; + lt;address type='pci' domain='0' bus='0' slot='4' function='0'
[libvirt] [PATCH 0/2] snapshot: add force for risky reverts
I first documented the need for force back in my RFC: https://www.redhat.com/archives/libvir-list/2011-August/msg00361.html but only now got around to implementing it. At the moment, I'm posting the code for early review. I'm still in the process of testing out multiple scenarios, and will send followup mail detailing the actual steps I took using virsh to cover the multiple scenarios. Eric Blake (2): snapshot: add REVERT_FORCE to API snapshot: enforce REVERT_FORCE on qemu include/libvirt/libvirt.h.in |1 + include/libvirt/virterror.h |2 + src/libvirt.c| 22 +++ src/qemu/qemu_driver.c | 47 +- src/util/virterror.c |6 + tools/virsh.c| 19 - tools/virsh.pod | 17 +++ 7 files changed, 103 insertions(+), 11 deletions(-) -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] snapshot: add REVERT_FORCE to API
Although reverting to a snapshot is a form of data loss, this is
normally expected. However, there are two cases where additional
surprises (failure to run the reverted state, or a break in
connectivity to the domain) can come into play. Requiring extra
acknowledgment in these cases will make it less likely that
someone can get into an unrecoverable state due to a default revert.
Also create a new error code, so users can distinguish when forcing
would make a difference, rather than having to blindly request force.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_REVERT_FORCE):
New flag.
* src/libvirt.c (virDomainRevertToSnapshot): Document it.
* include/libvirt/virterror.h (VIR_ERR_SNAPSHOT_REVERT_RISKY): New
error value.
* src/util/virterror.c (virErrorMsg): Implement it.
* tools/virsh.c (cmdDomainSnapshotRevert): Add --force to virsh.
* tools/virsh.pod (snapshot-revert): Document it.
---
include/libvirt/libvirt.h.in |1 +
include/libvirt/virterror.h |2 ++
src/libvirt.c| 22 ++
src/util/virterror.c |6 ++
tools/virsh.c| 19 ++-
tools/virsh.pod | 17 +
6 files changed, 66 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 3c7f278..7403a9a 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2736,6 +2736,7 @@ virDomainSnapshotPtr
virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
typedef enum {
VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 0, /* Run after revert */
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED = 1 1, /* Pause after revert */
+VIR_DOMAIN_SNAPSHOT_REVERT_FORCE = 1 2, /* Allow risky reverts */
} virDomainSnapshotRevertFlags;
/* Revert the domain to a point-in-time snapshot. The
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 0aae622..0c98014 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -237,6 +237,8 @@ typedef enum {
the given driver */
VIR_ERR_STORAGE_PROBE_FAILED = 75, /* storage pool proble failed */
VIR_ERR_STORAGE_POOL_BUILT = 76,/* storage pool already built */
+VIR_ERR_SNAPSHOT_REVERT_RISKY = 77, /* force was not requested for a
+ risky domain snapshot revert */
} virErrorNumber;
/**
diff --git a/src/libvirt.c b/src/libvirt.c
index 2b2f6be..c2aabf7 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -16373,6 +16373,28 @@ error:
* into an inactive state, so transient domains require the use of one
* of these two flags.
*
+ * Reverting to any snapshot discards all configuration changes made since
+ * the last snapshot. Additionally, reverting to a snapshot from a running
+ * domain is a form of data loss, since it discards whatever is in the
+ * guest's RAM at the time. However, the very nature of keeping snapshots
+ * implies the intent to roll back state, so no additional confirmation is
+ * normally required for these effects.
+ *
+ * However, there are two particular situations where reverting will
+ * be refused by default, and where @flags must include
+ * VIR_DOMAIN_SNAPSHOT_REVERT_FORCE to acknowledge the risks. 1) Any
+ * attempt to revert to a snapshot that lacks the metadata to perform
+ * ABI compatibility checks (generally the case for snapshots that
+ * lack a full domain when listed by virDomainSnapshotGetXMLDesc(),
+ * such as those created prior to libvirt 0.9.5). 2) Any attempt to
+ * revert a running domain to an active state that requires starting a
+ * new hypervisor instance rather than reusing the existing hypervisor
+ * (since this would terminate all connections to the domain, such as
+ * such as VNC or Spice graphics) - this condition arises from active
+ * snapshots that are provably ABI incomaptible, as well as from
+ * inactive snapshots with a request to start the domain after the
+ * revert.
+ *
* Returns 0 if the creation is successful, -1 on error.
*/
int
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 26c4981..5006fa2 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -1204,6 +1204,12 @@ virErrorMsg(virErrorNumber error, const char *info)
else
errmsg = _(argument unsupported: %s);
break;
+case VIR_ERR_SNAPSHOT_REVERT_RISKY:
+if (info == NULL)
+errmsg = _(revert requires force);
+else
+errmsg = _(revert requires force: %s);
+break;
}
return (errmsg);
}
diff --git a/tools/virsh.c b/tools/virsh.c
index 655378c..4e79325 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13571,6 +13571,7 @@ static const vshCmdOptDef opts_snapshot_revert[] = {
{snapshotname, VSH_OT_DATA, VSH_OFLAG_REQ, N_(snapshot name)},
{running, VSH_OT_BOOL, 0, N_(after reverting, change state to
running)},
[libvirt] [PATCH 2/2] snapshot: enforce REVERT_FORCE on qemu
Implements the documentation for snapshot revert vs. force.
Part of the patch tightens existing behavior (previously, reverting
to an old snapshot without domain was blindly attempted, now it
requires force), while part of it relaxes behavior (previously, it
was not possible to revert an active domain to an ABI-incompatible
active snapshot, now force allows this transition).
* src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Check for
risky situations, and allow force to get past them.
---
src/qemu/qemu_driver.c | 47 +--
1 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5110102..efd60a7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9753,7 +9753,8 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
* 7. paused - inactive: EVENT_STOPPED
* 8. paused - running: EVENT_RESUMED
* 9. paused - paused: none
- * Also, several transitions occur even if we fail partway through.
+ * Also, several transitions occur even if we fail partway through,
+ * and use of FORCE can cause multiple transitions.
*/
qemuDriverLock(driver);
@@ -9789,6 +9790,24 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
yet));
goto cleanup;
}
+if (!(flags VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
+if (!snap-def-dom) {
+qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
+_(snapshot lacks domain '%s' rollback details),
+snap-def-name);
+goto cleanup;
+}
+if (virDomainObjIsActive(vm)
+!(snap-def-state == VIR_DOMAIN_RUNNING
+ || snap-def-state == VIR_DOMAIN_PAUSED)
+(flags (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
+ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
+qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
+_(must respawn qemu to start inactive snapshot));
+goto cleanup;
+}
+}
+
if (vm-current_snapshot) {
vm-current_snapshot-def-current = false;
@@ -9818,11 +9837,6 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
VIR_FREE(xml);
if (!config)
goto cleanup;
-} else {
-/* XXX Fail if VIR_DOMAIN_REVERT_FORCE is not set, rather than
- * blindly hoping for the best. */
-VIR_WARN(snapshot is lacking rollback information for domain '%s',
- snap-def-name);
}
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
@@ -9843,10 +9857,22 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Transitions 5, 6, 8, 9 */
/* Check for ABI compatibility. */
if (config !virDomainDefCheckABIStability(vm-def, config)) {
-/* XXX Add VIR_DOMAIN_REVERT_FORCE to permit killing
- * and restarting a new qemu, since loadvm monitor
- * command won't work. */
-goto endjob;
+if (!(flags VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
+/* Alter existing error to give correct category. */
+virErrorPtr err = virGetLastError();
+err-code = VIR_ERR_SNAPSHOT_REVERT_RISKY;
+goto endjob;
+}
+qemuProcessStop(driver, vm, 0,
+VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT);
+virDomainAuditStop(vm, from-snapshot);
+detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
+event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ detail);
+if (event)
+qemuDomainEventQueue(driver, event);
+goto load;
}
priv = vm-privateData;
@@ -9882,6 +9908,7 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virDomainObjAssignDef(vm, config, false);
} else {
/* Transitions 2, 3 */
+load:
was_stopped = true;
if (config)
virDomainObjAssignDef(vm, config, false);
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] qemu: make PCI multifunction support more manual
On 09/30/2011 12:02 PM, Laine Stump wrote: (V2: address Daniel Veillard's two review points (don't allow multifunction='default', and add multifunction=off to the qemu commandline when that's what the XML says), and modify the checks for duplicate PCI address usage attempts to account for multifunction=off on a device's function 0, per IRC discussion with Dan Berrange (see new qemuCollectPCIAddress()). As a side effect, attempts to re-use the same PCI address are now logged rather than silently generating an error.) Yep, I can see those enhancements over v1. A side effect of this patch is that attempts to use the same PCI address for two different devices will now log an error (previously this would cause the domain define operation to fail, but there would be no log message generated). Because the function doing this log was almost completely rewritten, I didn't think it worthwhile to make a separate patch for that fix (the entire patch would immediately be obsoleted). Agree - no easy way to split it. @@ -1118,10 +1118,14 @@ Thecodetype/code attribute is mandatory, and is typically pci or drive. For a pci controller, additional attributes forcodebus/code,codeslot/code, -andcodefunction/code must be present, as well as an -optionalcodedomain/code. For a drive controller, -additional attributescodecontroller/code,codebus/code, +andcodefunction/code must be present, as well as +optionalcodedomain/code +andcodemultifunction/codespan class=sincesince +0.9.7, requires QEMU 0.13/span (defaults to 'off'). For a This wording makes it sound like both domain and multifunction were first introduced in 0.9.7, rather than the intended fact that 'multifunction' is the only newcomer attribute. Maybe reword along the lines of: ...as well as optional 'domain' and 'multifunction'. Multifunction defaults to 'off', any other value requires QEMU 0.13 and span class=sincelibvirt 0.9.7/span. +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 These are some rather long lines. While you are touching them, it would be worth splitting them with \-newline pairs to fit in more reasonable limits. ACK with those nits 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] Improve the daemon man page
On 09/30/2011 01:56 AM, Daniel Veillard wrote: Add informations about the handling of USR2 and fatal signals and document the default log location. diff --git a/daemon/libvirtd.pod.in b/daemon/libvirtd.pod.in index 6e699b8..269e81f 100644 --- a/daemon/libvirtd.pod.in +++ b/daemon/libvirtd.pod.in @@ -69,6 +69,10 @@ Display version information then exit. =head1 SIGNALS On receipt of BSIGHUP libvirtd will reload its configuration. +On receipt of BSIGUSR2 or other fatal signals like BSIGFPE, SIGUSR2 is not fatal. I'd word this: On receipt of BSIGUSR2, or on fatal signals like BSIGFPE, ... +BSIGSEGV, BSIGILL, BSIGABRT or BSIGBUS, libvirtd will dump +its internal debug message buffer content to the default log output, +usually B@localstatedir@/log/libvirt/libvirtd.log =head1 FILES @@ -105,6 +109,11 @@ The TLS BServer private key libvirtd will use. The PID file to use, unless overridden by the B-p|B--pid-file option. +=item F@localstatedir@/log/libvirt/libvirtd.log + +The default file where libvirtd will save its logs, this can be overriden s/overriden/overridden/ +from the B@sysconfdir@/libvirtd.conf configuration file. ACK with nits 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] bug: try to take disk snapshot for LVM2 Volume
On 09/29/2011 11:26 PM, MATSUDA, Daiki wrote: I tried the new snapshot function implemented by Eric Blake. It works very well for QCOW2 disk image system. But I often use LVM2 volume for QEMU virtual machines and tried to take disk snapshot by virsh command ( snapshot-create DOMNAME --disk-only). So, finally qemu monitor command 'snapshot_blkdev' accepts the LVM2 volume and create QCOW2 snapshot image. In addition, domain's configuration file is replaced to use snapshot disk image instead of LVM2 volume. It sounds like virsh did what it was told, but that you told it so little information that it had to fill in the gaps and choose its own destination file name (hence the .1317357844 suffix after the action). Your situation sounds like a case where you may want a bit more control over the destination file name. configuration file from disk type='block' device='disk driver name='qemu' type='raw' cache='none'/ source dev='dev/VG1/LVM2_dom'/ to disk type='block' device='disk driver name='qemu' type='qcow2' cache='none'/ source dev='dev/VG1/LVM2_dom.1317357844'/ Are you pasting literal chunks, or retyping this? You're missing the / in front of dev/VG1, so I can't help but wonder what else might have been mistyped. After then, the domain runs well till it is shutdowned. I'm surprised that it got that far - generally, libvirt can't create random regular files under the /dev/VG1/ device-mapper hierarchy, and if a file can't be created, then what was open() doing, and what did qemu actually do? It may be worth looking at lsof says that qemu has open, if you still have it running. Or it may be that you've found a bug in libvirt and/or qemu for not accurately reporting failure to create the snapshot image. I think we need to step back a bit and look at the bigger picture. Do you want your new qcow2 file to be its own LVM volume (in which case, I'd suggest that you pre-create the LVM volume, and pass in the file name within /dev/VG1/ of the existing block device to use), or are you okay with it being a regular file (in which case, I'd suggest that you do not pre-create the file, but that you still pass in the desired filename to some absolute path that lives outside of /dev/)? Either way, it sounds like you do _not_ want libvirt to be generating its own filename, since libvirt only knows how to generate a name in the same directory as the snapshot is taking place, but your lvm is in a special directory. To do this, you either need to create an XML file yourself, and call 'virsh snapshot-create dom --disk-only file', or you need to have virsh create the xml for you with 'virsh snapshot-create-as dom --disk-only vda,file=/path/to/file'. You can see the xml that snapshot-create-as would generate (in case you need to further fine-tune it for your own use in snapshot-create) via the --print-xml option. I started the domain, but it does not with following error virtsh # start LVM2_dom error: Failed to start domain LVM2_dom error: 内部エラー Process exited while reading console log output: char device redirected to /dev/pts/7 qemu: could not open disk image /dev/VG1/LVM2_dom.1317357844: Invalid argument. That makes sense, if that file doesn't exist (but why qemu didn't reject the snapshot in the first place still remains to be investigated). I think that if the volume but qcow2 is given libvirt should be refuse, No, qemu does just fine with a non-qcow2 backing file. The real problem here is that the new qcow2 file was not created, not the type of the original file. e.g. in qemuDomainSnapshotCreateDiskActive() with voulme driver type. But currently the structures concerning with snapshot or disk has no member to hold such a volume driver information. In addition, as we want to add the LVM2 and other volume snapshot function, we hope you add its information and fix. Yes, I have much longer-term plans for refactoring device snapshots to move the work into more virStorageVolPtr operations, and teach virDomainSnapshotCreateXML to reuse virStorageVol actions rather than hard-coding the actions itself, at which point we can then use lvm snapshots rather than qcow2 snapshots. But that's a lot more effort, so no promise of how long it will take me to get to that point. -- 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/2] Make libvirt.so include the RPC server code
On 09/30/2011 07:54 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com To avoid static linking libvirtd to the RPC server code, which then prevents sane introduction of DTrace probes, put it all in the libvirt.so, and export it * daemon/Makefile.am: Don't link to RPC libraries * src/Makefile.am: Link all RPC libraries to libvirt.so * src/libvirt_private.syms: Export all RPC functions --- daemon/Makefile.am |2 - src/Makefile.am |2 +- src/libvirt_private.syms | 76 ++ 3 files changed, 77 insertions(+), 3 deletions(-) +++ b/src/Makefile.am @@ -612,7 +612,7 @@ libvirt_driver_remote_la_CFLAGS = \ -I@top_srcdir@/src/rpc \ $(AM_CFLAGS) libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) libvirt-net-rpc-client.la libvirt-net-rpc.la +libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) libvirt-net-rpc-client.la libvirt-net-rpc-server.la libvirt-net-rpc.la Use \-newline wrapping to make this not be quite so long. if WITH_DRIVER_MODULES libvirt_driver_remote_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_remote_la_LDFLAGS += -module -avoid-version diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2a3fab..e086253 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1162,6 +1162,82 @@ virFileDirectFdNew; virFileFclose; virFileFdopen; +# rpc Hmm, that's not really the .h where they are declared, although it is the right directory. As is, sticking 'rpc' between 'virfile.h' and 'virpidfile.h' doesn't sort very well. It shouldn't be too much more effort to break this into multiple sections, one for each relevant .h... virnetmessage.h: +virNetMessageClear; +virNetMessageEncodeHeader; +virNetMessageEncodePayload; +virNetMessageFree; +virNetMessageNew; +virNetMessageQueuePush; +virNetMessageQueueServe; +virNetMessageSaveError; virnetsaslcontext.h +virNetSASLContextCheckIdentity; +virNetSASLContextNewServer; +virNetSASLSessionExtKeySize; +virNetSASLSessionFree; +virNetSASLSessionGetIdentity; +virNetSASLSessionGetKeySize; +virNetSASLSessionListMechanisms; +virNetSASLSessionNewServer; +virNetSASLSessionSecProps; +virNetSASLSessionServerStart; +virNetSASLSessionServerStep; virnetserver.h +virNetServerAddProgram; +virNetServerAddService; +virNetServerAddSignalHandler; +virNetServerAutoShutdown; virnetserverclient.h +virNetServerClientAddFilter; +virNetServerClientClose; +virNetServerClientDelayedClose; +virNetServerClientFree; +virNetServerClientGetAuth; +virNetServerClientGetFD; +virNetServerClientGetLocalIdentity; +virNetServerClientGetPrivateData; +virNetServerClientGetReadonly; +virNetServerClientGetTLSKeySize; +virNetServerClientHasTLSSession; +virNetServerClientImmediateClose; +virNetServerClientIsSecure; +virNetServerClientLocalAddrString; +virNetServerClientRef; +virNetServerClientRemoteAddrString; +virNetServerClientRemoveFilter; +virNetServerClientSendMessage; +virNetServerClientSetCloseHook; +virNetServerClientSetIdentity; +virNetServerClientSetPrivateData; +virNetServerClientSetSASLSession; move these to be with the rest of virnetserver.h +virNetServerClose; +virNetServerFree; +virNetServerIsPrivileged; +virNetServerNew; virnetserverprogram.h +virNetServerProgramFree; +virNetServerProgramGetID; +virNetServerProgramGetVersion; +virNetServerProgramMatches; +virNetServerProgramNew; +virNetServerProgramRef; +virNetServerProgramSendReplyError; +virNetServerProgramSendStreamData; +virNetServerProgramSendStreamError; +virNetServerQuit; +virNetServerRef; +virNetServerRun; virnetserverservice.h +virNetServerServiceFree; +virNetServerServiceNewTCP; +virNetServerServiceNewUNIX; +virNetServerUpdateServices; virnetsocket.h +virNetSocketDupFD; +virNetSocketFree; +virNetSocketGetFD; +virNetSocketListen; +virNetSocketNewConnectTCP; +virNetSocketNewListenUNIX; virnettlscontext.h +virNetTLSContextFree; +virNetTLSContextNewServer; +virNetTLSContextNewServerPath; + # virpidfile.h virPidFileAcquire; But all of those are just layout nits, not technical objections, so I'm okay giving: ACK with changes -- 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 2/2] Add some DTrace/SystemTAP probes to the RPC code
On 09/30/2011 07:54 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Not much of a commit comment. Even mentioning that half of the patch is code motion rather than new content might be useful. + +/* Systemtap 1.2 headers have a bug where they cannot handle a + * variable declared with array type. Work around this by casting all + * arguments. This is some gross use of the preprocessor because + * PROBE is a var-arg macro, but it is better than the alternative of + * making all callers to PROBE have to be aware of the issues. And + * hopefully, if we ever add a call to PROBE with other than 2 or 3 s/other than 2 or 3/more than 7/ + * end arguments, you can figure out the pattern to extend this hack. + */ +# define VIR_COUNT_ARGS(...) VIR_ARG9(__VA_ARGS__, 8, 7, 6, 5, 4, 3, 2, 1) +# define VIR_ARG9(_1, _2, _3, _4, _5, _6, _7, _8, _9, ...) _9 +# define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__) +# define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__) + +/* The double cast is necessary to silence gcc warnings; any pointer + * can safely go to intptr_t and back to void *, which collapses + * arrays into pointers; while any integer can be widened to intptr_t + * then cast to void *. */ +# define VIR_ADD_CAST(a) ((void *)(intptr_t)(a)) Now that you've expanded the list, it looks awkward not having VIR_ADD_CAST1(a) VIR_ADD_CAST(a) +# define VIR_ADD_CAST2(a, b) \ +VIR_ADD_CAST(a), VIR_ADD_CAST(b) +# define VIR_ADD_CAST3(a, b, c)\ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c) +# define VIR_ADD_CAST4(a, b, c, d) \ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),\ +VIR_ADD_CAST(d) +# define VIR_ADD_CAST5(a, b, c, d, e)\ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),\ +VIR_ADD_CAST(d), VIR_ADD_CAST(e) +# define VIR_ADD_CAST6(a, b, c, d, e, f) \ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),\ +VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f) +# define VIR_ADD_CAST7(a, b, c, d, e, f, g) \ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c), \ +VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f), \ +VIR_ADD_CAST(g) Up to here is reachable... +# define VIR_ADD_CAST8(a, b, c, d, e, f, g, h) \ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),\ +VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f), \ +VIR_ADD_CAST(g), VIR_ADD_CAST(h) +# define VIR_ADD_CAST9(a, b, c, d, e, f, g, h, i)\ +VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c), \ +VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f), \ +VIR_ADD_CAST(g), VIR_ADD_CAST(h), VIR_ADD_CAST(i) but these two are impossible given your definition of VIR_COUNT_ARGS. To really support 9 arguments, VIR_COUNT_ARGS needs to call VIR_ARG11(__VA_ARGS__, 10, 9, ...). ACK with that fixed. Everything else looks like a lot of stap black magic, but the end result is indeed a setup that can be easily traced to track traffic. -- 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] RPM build failure on Redhat6
On 09/28/2011 10:48 PM, Wayne Xia wrote: Hi, I want to build rpm for latest libvit version on Redhat6, but when I type: ./autogen.sh --system make rpm -j4 error happens: sitemap.html.tmp index.html.tmp archdomain.html.tmp downloads.html.tmp auth.html.tmp internals/locking.html.tmp archnode.html.tmp hvsupport.html.tmp bugs.html.tmp java.html.tmp locking.html.tmp logging.html.tmp testapi.html.tmp formatnetwork.html.tmp bindings.html.tmp uri.html.tmp drvtest.html.tmp contact.html.tmp formatnwfilter.html.tmp intro.html.tmp drvxen.html.tmp drvesx.html.tmp drvqemu.html.tmp drvvmware.html.tmp testsuites.html.tmp devguide.html.tmp firewall.html.tmp architecture.html.tmp todo.html.tmp formatsnapshot.html.tmp formatstorage.html.tmp deployment.html.tmp internals.html.tmp api_extension.html.tmp drvuml.html.tmp virshcmdref.html.tmp csharp.html.tmp python.html.tmp drvremote.html.tmp archstorage.html.tmp remote.html.tmp drivers.html.tmp formatnode.html.tmp make[2]: Leaving directory `/home/xiawenc/WorkDir/Source/libvirt/libvirt/docs' make[1]: *** [distdir] Error 1 That's not the actual error (which happened several lines before where you started pasting). I'm not sure what's going on, but I will add that I have sometimes seen a 'make -j2' failure when the makefile gets to the point of generating todo.html. I'm not sure where the missing make dependency is, but hopefully next time someone sees it, and accurately reports it, we can patch docs/Makefile.am to avoid these parallel make problems. In the meantime, 'make -C docs' without -j should get you past this issue. -- 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/1] lvm storage backend: handle command_names=1 in lvm.conf (v2)
On 09/28/2011 02:08 PM, Serge E. Hallyn wrote:
[ Thanks for the feedback, Eric. Hopefully I correctly incorporated it
in this version. This version still tests fine with and without
lvm.conf command_names=1 ]
Glad that it passed testing (as I had not tested my suggestions).
If the regexes supported (?:pvs)?, then we could handle this by
optionally matching but not returning the initial command name. But it
doesn't. So add a new char* argument to
virStorageBackendRunProgRegex(). If that argument is NULL then we act
as usual. Otherwise, if the string at that argument is found at the
start of a returned line, we drop that before running the regex.
With this patch, virt-manager shows me lvs with command_names 1 or 0.
The definitions of PVS_BASE etc may want to be moved into the configure
scripts (though given how PVS is found, IIUC that could only happen if
pvs was a link to pvs_real), but in any case no sense dealing with that
until we're sure this is an ok way to handle it.
Changelog:
Sep 28: Use STRSKIP and make cmd_to_ignore arg const, as per Eric's
feedback.
I'll tweak the commit a bit (information like this is useful to the
patch review, but less useful in 'git log' - so the best place to stick
it is after the --- divider, so that 'git am' will know that it is not
part of the official commit message).
@@ -1460,13 +1460,20 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool,
}
while (fgets(line, sizeof(line), list) != NULL) {
+char *p = NULL;
/* Strip trailing newline */
int len = strlen(line);
if (len line[len-1] == '\n')
line[len-1] = '\0';
+/* if cmd_to_ignore is specified, then ignore it */
I'll tweak this comment slightly to mention that we are skipping a
prefix, if it is present, and not ignoring the entire command line.
+++ b/src/storage/storage_backend_logical.c
@@ -205,13 +205,14 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
pool-def-source.name, NULL
};
+#define LVS_BASE lvs
if (virStorageBackendRunProgRegex(pool,
prog,
1,
regexes,
vars,
virStorageBackendLogicalMakeVol,
- vol) 0) {
+ vol, LVS_BASE) 0) {
I'm not a big fan of in-function #define, especially if it's for a
one-shot string literal. I generally either float the #define up to the
top of the file (out of the function), or in this case, inline the
string constant into its lone usage point (we can refactor into a
#define later, if we need to use the string more than once).
ACK and pushed with this squashed in:
diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c
index dd7e36b..64c35c2 100644
--- i/src/storage/storage_backend.c
+++ w/src/storage/storage_backend.c
@@ -1400,7 +1400,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr
pool,
const char **regex,
int *nvars,
virStorageBackendListVolRegexFunc func,
- void *data, const char *cmd_to_ignore)
+ void *data, const char *prefix)
{
int fd = -1, err, ret = -1;
FILE *list = NULL;
@@ -1466,9 +1466,9 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr
pool,
if (len line[len-1] == '\n')
line[len-1] = '\0';
-/* if cmd_to_ignore is specified, then ignore it */
-if (cmd_to_ignore)
-p = STRSKIP(line, cmd_to_ignore);
+/* ignore any command prefix */
+if (prefix)
+p = STRSKIP(line, prefix);
if (!p)
p = line;
diff --git i/src/storage/storage_backend_logical.c
w/src/storage/storage_backend_logical.c
index 7923b71..589f486 100644
--- i/src/storage/storage_backend_logical.c
+++ w/src/storage/storage_backend_logical.c
@@ -205,14 +205,13 @@
virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
pool-def-source.name, NULL
};
-#define LVS_BASE lvs
if (virStorageBackendRunProgRegex(pool,
prog,
1,
regexes,
vars,
virStorageBackendLogicalMakeVol,
- vol, LVS_BASE) 0) {
+ vol, lvs) 0) {
return -1;
}
@@ -328,10 +327,9 @@
virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
memset(sourceList, 0, sizeof(sourceList));
sourceList.type = VIR_STORAGE_POOL_LOGICAL;
-#define PVS_BASE pvs
if (virStorageBackendRunProgRegex(NULL, prog, 1,
[libvirt] [libvirt-glib] gir: fix introspection of asyncs and array delegate
---
libvirt-gobject/libvirt-gobject-connection.c | 24
libvirt-gobject/libvirt-gobject-connection.h |6 +++---
libvirt-gobject/libvirt-gobject-storage-pool.c |8
libvirt-gobject/libvirt-gobject-storage-pool.h |2 +-
libvirt-gobject/libvirt-gobject-stream.h |2 +-
5 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-connection.c
b/libvirt-gobject/libvirt-gobject-connection.c
index 90d1566..5fc0a9e 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -431,19 +431,19 @@ gvir_connection_open_helper(GSimpleAsyncResult *res,
* gvir_connection_open_async:
* @conn: the connection
* @cancellable: (allow-none)(transfer none): cancellation object
- * @callback: (transfer none): completion callback
- * @opaque: (transfer none)(allow-none): opaque data for callback
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
*/
void gvir_connection_open_async(GVirConnection *conn,
GCancellable *cancellable,
GAsyncReadyCallback callback,
-gpointer opaque)
+gpointer user_data)
{
GSimpleAsyncResult *res;
res = g_simple_async_result_new(G_OBJECT(conn),
callback,
-opaque,
+user_data,
gvir_connection_open);
g_simple_async_result_run_in_thread(res,
gvir_connection_open_helper,
@@ -828,19 +828,19 @@ gvir_connection_fetch_domains_helper(GSimpleAsyncResult
*res,
* gvir_connection_fetch_domains_async:
* @conn: the connection
* @cancellable: (allow-none)(transfer none): cancellation object
- * @callback: (transfer none): completion callback
- * @opaque: (transfer none)(allow-none): opaque data for callback
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
*/
void gvir_connection_fetch_domains_async(GVirConnection *conn,
GCancellable *cancellable,
GAsyncReadyCallback callback,
- gpointer opaque)
+ gpointer user_data)
{
GSimpleAsyncResult *res;
res = g_simple_async_result_new(G_OBJECT(conn),
callback,
-opaque,
+user_data,
gvir_connection_fetch_domains);
g_simple_async_result_run_in_thread(res,
gvir_connection_fetch_domains_helper,
@@ -889,19 +889,19 @@ gvir_connection_fetch_pools_helper(GSimpleAsyncResult
*res,
* gvir_connection_fetch_storage_pools_async:
* @conn: the connection
* @cancellable: (allow-none)(transfer none): cancellation object
- * @callback: (transfer none): completion callback
- * @opaque: (transfer none)(allow-none): opaque data for callback
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
*/
void gvir_connection_fetch_storage_pools_async(GVirConnection *conn,
GCancellable *cancellable,
GAsyncReadyCallback callback,
- gpointer opaque)
+ gpointer user_data)
{
GSimpleAsyncResult *res;
res = g_simple_async_result_new(G_OBJECT(conn),
callback,
-opaque,
+user_data,
gvir_connection_fetch_storage_pools);
g_simple_async_result_run_in_thread(res,
gvir_connection_fetch_pools_helper,
diff --git a/libvirt-gobject/libvirt-gobject-connection.h
b/libvirt-gobject/libvirt-gobject-connection.h
index d05f792..2eb58ec 100644
--- a/libvirt-gobject/libvirt-gobject-connection.h
+++ b/libvirt-gobject/libvirt-gobject-connection.h
@@ -77,7 +77,7 @@ gboolean gvir_connection_open(GVirConnection *conn,
void gvir_connection_open_async(GVirConnection *conn,
GCancellable *cancellable,
GAsyncReadyCallback callback,
-gpointer opaque);
+gpointer user_data);
gboolean gvir_connection_open_finish(GVirConnection *conn,
GAsyncResult *result,
GError **err);
@@ -90,7 +90,7 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn,
Re: [libvirt] [PATCH 2.6/4] snapshot: virsh fallback for snapshot-list --from children
On 09/29/2011 04:02 PM, Eric Blake wrote:
Iterating over one level of children requires parsing all snapshots
and their parents; a bit of code shuffling makes it pretty easy
to do this as well.
* tools/virsh.c (cmdSnapshotList): Add another fallback.
---
Applies anywhere after 2.5/4, but again, easiest to test against the
same version of libvirtd if applied before 3/4.
+if (numsnaps 0) {
+/* XXX also want to emulate --descendants without --tree */
+if ((!descendants || tree)
+last_error-code == VIR_ERR_NO_SUPPORT) {
This doesn't even compile. Let me send a cleaned up v2.
--
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
[libvirt] [PATCHv2 3/7] snapshot: virsh fallback for snapshot-list --tree --from
Emulating --from requires grabbing the entire list of snapshots
and their parents, and recursively iterating over the list from
the point of interest - but we already do that for --tree. This
turns on emulation for that situation.
* tools/virsh.c (__vshControl): Rename member.
(vshReconnect, cmdConnect, vshGetSnapshotParent): Update clients.
(cmdSnapshotList): Add fallback.
---
v2: reuse existing ctl flag, so that virsh in shell mode doesn't
repeatedly try non-working commands
tools/virsh.c | 43 +--
1 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index adafe86..e3bc364 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -246,8 +246,8 @@ typedef struct __vshControl {
char *historyfile; /* readline history file name */
bool useGetInfo;/* must use virDomainGetInfo, since
virDomainGetState is not supported */
-bool useSnapshotGetXML; /* must use virDomainSnapshotGetXMLDesc, since
- virDomainSnapshotGetParent is missing */
+bool useSnapshotOld;/* cannot use virDomainSnapshotGetParent or
+ virDomainSnapshotNumChildren */
} __vshControl;
typedef struct vshCmdGrp {
@@ -599,7 +599,7 @@ vshReconnect(vshControl *ctl)
vshError(ctl, %s, _(Reconnected to the hypervisor));
disconnected = 0;
ctl-useGetInfo = false;
-ctl-useSnapshotGetXML = false;
+ctl-useSnapshotOld = false;
}
/* ---
@@ -747,7 +747,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
ctl-name = vshStrdup(ctl, name);
ctl-useGetInfo = false;
-ctl-useSnapshotGetXML = false;
+ctl-useSnapshotOld = false;
ctl-readonly = ro;
ctl-conn = virConnectOpenAuth(ctl-name, virConnectAuthPtrDefault,
@@ -13042,7 +13042,7 @@ vshGetSnapshotParent(vshControl *ctl,
virDomainSnapshotPtr snapshot,
*parent_name = NULL;
/* Try new API, since it is faster. */
-if (!ctl-useSnapshotGetXML) {
+if (!ctl-useSnapshotOld) {
parent = virDomainSnapshotGetParent(snapshot, 0);
if (parent) {
/* API works, and virDomainSnapshotGetName will be accurate */
@@ -13056,7 +13056,7 @@ vshGetSnapshotParent(vshControl *ctl,
virDomainSnapshotPtr snapshot,
goto cleanup;
}
/* API didn't work, fall back to XML scraping. */
-ctl-useSnapshotGetXML = true;
+ctl-useSnapshotOld = true;
}
xml = virDomainSnapshotGetXMLDesc(snapshot, 0);
@@ -13181,10 +13181,22 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, descendants) || tree) {
flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
}
-numsnaps = virDomainSnapshotNumChildren(start, flags);
-if (numsnaps = 0 tree)
-numsnaps++;
-/* XXX Is it worth emulating --from on older servers? */
+numsnaps = ctl-useSnapshotOld ? -1 :
+virDomainSnapshotNumChildren(start, flags);
+/* XXX Is it worth emulating --from without --tree on older servers?
*/
+if (tree) {
+if (numsnaps = 0) {
+numsnaps++;
+} else if (ctl-useSnapshotOld ||
+ last_error-code == VIR_ERR_NO_SUPPORT) {
+/* We can emulate --tree --from. */
+virFreeError(last_error);
+last_error = NULL;
+ctl-useSnapshotOld = true;
+flags = ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+numsnaps = virDomainSnapshotNum(dom, flags);
+}
+}
} else {
numsnaps = virDomainSnapshotNum(dom, flags);
@@ -13199,8 +13211,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
}
}
-if (numsnaps 0)
+if (numsnaps 0) {
+if (!last_error)
+vshError(ctl, _(missing support));
goto cleanup;
+}
if (!tree) {
if (parent_filter 0)
@@ -13222,7 +13237,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (VIR_ALLOC_N(names, numsnaps) 0)
goto cleanup;
-if (from) {
+if (from !ctl-useSnapshotOld) {
/* When mixing --from and --tree, we want to start the tree at the
* given snapshot. Without --tree, only list the children. */
if (tree) {
@@ -13247,7 +13262,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (tree) {
char indentBuf[INDENT_BUFLEN];
char **parents = vshCalloc(ctl, sizeof(char *), actual);
-for (i = (from ? 1 : 0); i actual; i++) {
+for (i = (from !ctl-useSnapshotOld); i actual; i++) {
/* free up memory from previous iterations of the loop */
if (snapshot)
virDomainSnapshotFree(snapshot);
@@ -13262,7 +13277,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
}
[libvirt] [PATCHv2 0/7] snapshot: listing children
Cleaned up the rebase goofs present throughout my v1: https://www.redhat.com/archives/libvir-list/2011-September/msg01270.html Eric Blake (7): snapshot: new virDomainSnapshotListChildrenNames API snapshot: virsh snapshot-list and children snapshot: virsh fallback for snapshot-list --tree --from snapshot: virsh fallback for snapshot-list --from children snapshot: virsh fallback for snapshot-list --descendants --from snapshot: remote protocol for snapshot children snapshot: implement snapshot children listing in qemu include/libvirt/libvirt.h.in| 27 +-- python/generator.py |4 + python/libvirt-override-api.xml | 12 ++- python/libvirt-override.c | 45 + src/conf/domain_conf.c | 51 +++ src/conf/domain_conf.h |7 ++ src/driver.h| 12 +++ src/libvirt.c | 111 +++ src/libvirt_private.syms|2 + src/libvirt_public.syms |2 + src/qemu/qemu_driver.c | 87 ++ src/remote/remote_driver.c |2 + src/remote/remote_protocol.x| 25 +- src/remote_protocol-structs | 20 tools/virsh.c | 190 -- tools/virsh.pod |9 ++- 16 files changed, 564 insertions(+), 42 deletions(-) -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/7] snapshot: new virDomainSnapshotListChildrenNames API
The previous API addition allowed traversal up the hierarchy;
this one makes it easier to traverse down the hierarchy.
In the python bindings, virDomainSnapshotNumChildren can be
generated, but virDomainSnapshotListChildrenNames had to copy
from the hand-written example of virDomainSnapshotListNames.
* include/libvirt/libvirt.h.in (virDomainSnapshotNumChildren)
(virDomainSnapshotListChildrenNames): New prototypes.
(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS): New flag alias.
* src/libvirt.c (virDomainSnapshotNumChildren)
(virDomainSnapshotListChildrenNames): New functions.
* src/libvirt_public.syms: Export them.
* src/driver.h (virDrvDomainSnapshotNumChildren)
(virDrvDomainSnapshotListChildrenNames): New callbacks.
* python/generator.py (skip_impl, nameFixup): Update lists.
* python/libvirt-override-api.xml: Likewise.
* python/libvirt-override.c
(libvirt_virDomainSnapshotListChildrenNames): New wrapper function.
---
v2: no change
include/libvirt/libvirt.h.in| 27 +++--
python/generator.py |4 ++
python/libvirt-override-api.xml | 12 +++-
python/libvirt-override.c | 45
src/driver.h| 12
src/libvirt.c | 111 +++
src/libvirt_public.syms |2 +
7 files changed, 204 insertions(+), 9 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a832b65..7403a9a 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2686,13 +2686,19 @@ virDomainSnapshotPtr
virDomainSnapshotCreateXML(virDomainPtr domain,
char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
unsigned int flags);
-/* Flags valid for both virDomainSnapshotNum() and
- * virDomainSnapshotListNames(). */
+/* Flags valid for virDomainSnapshotNum(),
+ * virDomainSnapshotListNames(), virDomainSnapshotNumChildren(), and
+ * virDomainSnapshotListChildrenNames(). Note that the interpretation
+ * of flag (10) depends on which function it is passed to. */
typedef enum {
-VIR_DOMAIN_SNAPSHOT_LIST_ROOTS= (1 0), /* Filter by snapshots which
- have no parents */
-VIR_DOMAIN_SNAPSHOT_LIST_METADATA = (1 1), /* Filter by snapshots which
- have metadata */
+VIR_DOMAIN_SNAPSHOT_LIST_ROOTS = (1 0), /* Filter by snapshots
+with no parents, when
+listing a domain */
+VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS = (1 0), /* List all descendants,
+not just children, when
+listing a snapshot */
+VIR_DOMAIN_SNAPSHOT_LIST_METADATA= (1 1), /* Filter by snapshots
+which have metadata */
} virDomainSnapshotListFlags;
/* Return the number of snapshots for this domain */
@@ -2702,6 +2708,15 @@ int virDomainSnapshotNum(virDomainPtr domain, unsigned
int flags);
int virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
unsigned int flags);
+/* Return the number of child snapshots for this snapshot */
+int virDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
+ unsigned int flags);
+
+/* Get the names of all child snapshots for this snapshot */
+int virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
+ char **names, int nameslen,
+ unsigned int flags);
+
/* Get a handle to a named snapshot */
virDomainSnapshotPtr virDomainSnapshotLookupByName(virDomainPtr domain,
const char *name,
diff --git a/python/generator.py b/python/generator.py
index 79558dd..71afdb7 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -352,6 +352,7 @@ skip_impl = (
'virConnectListDefinedInterfaces',
'virConnectListNWFilters',
'virDomainSnapshotListNames',
+'virDomainSnapshotListChildrenNames',
'virConnGetLastError',
'virGetLastError',
'virDomainGetInfo',
@@ -963,6 +964,9 @@ def nameFixup(name, classe, type, file):
elif name[0:26] == virDomainSnapshotListNames:
func = name[9:]
func = string.lower(func[0:1]) + func[1:]
+elif name[0:28] == virDomainSnapshotNumChildren:
+func = name[17:]
+func = string.lower(func[0:1]) + func[1:]
elif name[0:20] == virDomainSnapshotNum:
func = name[9:]
func = string.lower(func[0:1]) + func[1:]
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 3013e46..ef02f34 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@
[libvirt] [PATCHv2 4/7] snapshot: virsh fallback for snapshot-list --from children
Iterating over one level of children requires parsing all snapshots
and their parents; a bit of code shuffling makes it pretty easy
to do this as well.
* tools/virsh.c (cmdSnapshotList): Add another fallback.
---
v2: fix compilation error, rebase around ctl flag
tools/virsh.c | 48
1 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index e3bc364..b2523d3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13117,6 +13117,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
information needed, 1 for parent column */
int numsnaps;
char **names = NULL;
+char **parents = NULL;
int actual = 0;
int i;
xmlDocPtr xml = NULL;
@@ -13132,6 +13133,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
bool tree = vshCommandOptBool(cmd, tree);
const char *from = NULL;
virDomainSnapshotPtr start = NULL;
+bool descendants = false;
if (vshCommandOptString(cmd, from, from) 0) {
vshError(ctl, _(invalid from argument '%s'), from);
@@ -13175,27 +13177,29 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if (from) {
+descendants = vshCommandOptBool(cmd, descendants);
start = virDomainSnapshotLookupByName(dom, from, 0);
if (!start)
goto cleanup;
-if (vshCommandOptBool(cmd, descendants) || tree) {
+if (descendants || tree) {
flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
}
numsnaps = ctl-useSnapshotOld ? -1 :
virDomainSnapshotNumChildren(start, flags);
-/* XXX Is it worth emulating --from without --tree on older servers?
*/
-if (tree) {
-if (numsnaps = 0) {
-numsnaps++;
-} else if (ctl-useSnapshotOld ||
- last_error-code == VIR_ERR_NO_SUPPORT) {
-/* We can emulate --tree --from. */
+if (numsnaps 0) {
+/* XXX also want to emulate --descendants without --tree */
+if ((!descendants || tree)
+(ctl-useSnapshotOld ||
+ last_error-code == VIR_ERR_NO_SUPPORT)) {
+/* We can emulate --from. */
virFreeError(last_error);
last_error = NULL;
ctl-useSnapshotOld = true;
flags = ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
numsnaps = virDomainSnapshotNum(dom, flags);
}
+} else if (tree) {
+numsnaps++;
}
} else {
numsnaps = virDomainSnapshotNum(dom, flags);
@@ -13259,9 +13263,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (actual 0)
goto cleanup;
-if (tree) {
-char indentBuf[INDENT_BUFLEN];
-char **parents = vshCalloc(ctl, sizeof(char *), actual);
+if (!tree)
+qsort(names[0], actual, sizeof(char*), namesorter);
+
+if (tree || ctl-useSnapshotOld) {
+parents = vshCalloc(ctl, sizeof(char *), actual);
for (i = (from !ctl-useSnapshotOld); i actual; i++) {
/* free up memory from previous iterations of the loop */
if (snapshot)
@@ -13275,6 +13281,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
}
+}
+if (tree) {
+char indentBuf[INDENT_BUFLEN];
for (i = 0 ; i actual ; i++) {
memset(indentBuf, '\0', sizeof indentBuf);
if (ctl-useSnapshotOld ? STREQ(names[i], from) : !parents[i])
@@ -13288,16 +13297,19 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
0,
indentBuf);
}
-for (i = 0 ; i actual ; i++)
-VIR_FREE(parents[i]);
-VIR_FREE(parents);
ret = true;
goto cleanup;
} else {
-qsort(names[0], actual, sizeof(char*), namesorter);
+if (ctl-useSnapshotOld descendants) {
+/* XXX emulate --descendants as well */
+goto cleanup;
+}
for (i = 0; i actual; i++) {
+if (ctl-useSnapshotOld STRNEQ_NULLABLE(parents[i], from))
+continue;
+
/* free up memory from previous iterations of the loop */
VIR_FREE(parent);
VIR_FREE(state);
@@ -13362,9 +13374,13 @@ cleanup:
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
VIR_FREE(doc);
-for (i = 0; i actual; i++)
+for (i = 0; i actual; i++) {
VIR_FREE(names[i]);
+if (parents)
+VIR_FREE(parents[i]);
+}
VIR_FREE(names);
+VIR_FREE(parents);
if (dom)
virDomainFree(dom);
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/7] snapshot: virsh snapshot-list and children
Sometimes, we only care about one branch of the snapshot hierarchy.
Make it easier to list a single branch, by using the new APIs.
Technically, I could emulate these new virsh options on old servers
by doing a complete dump, then scraping xml to filter out just the
snapshots that I care about, but I didn't want to do that in this patch.
* tools/virsh.c (cmdSnapshotList): Add --from, --descendants.
* tools/virsh.pod (snapshot-list): Document them.
---
v2: minor rebase cleanups
tools/virsh.c | 76 ---
tools/virsh.pod |9 ++-
2 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 955b8df..adafe86 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13102,6 +13102,8 @@ static const vshCmdOptDef opts_snapshot_list[] = {
{metadata, VSH_OT_BOOL, 0,
N_(list only snapshots that have metadata that would prevent undefine)},
{tree, VSH_OT_BOOL, 0, N_(list snapshots in a tree)},
+{from, VSH_OT_DATA, 0, N_(limit list to children of given snapshot)},
+{descendants, VSH_OT_BOOL, 0, N_(with --from, list all descendants)},
{NULL, 0, 0, NULL}
};
@@ -13128,25 +13130,36 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
char timestr[100];
struct tm time_info;
bool tree = vshCommandOptBool(cmd, tree);
+const char *from = NULL;
+virDomainSnapshotPtr start = NULL;
+
+if (vshCommandOptString(cmd, from, from) 0) {
+vshError(ctl, _(invalid from argument '%s'), from);
+goto cleanup;
+}
if (vshCommandOptBool(cmd, parent)) {
if (vshCommandOptBool(cmd, roots)) {
vshError(ctl, %s,
- _(--parent and --roots are mutually exlusive));
+ _(--parent and --roots are mutually exclusive));
return false;
}
if (tree) {
vshError(ctl, %s,
- _(--parent and --tree are mutually exlusive));
+ _(--parent and --tree are mutually exclusive));
return false;
}
parent_filter = 1;
} else if (vshCommandOptBool(cmd, roots)) {
if (tree) {
vshError(ctl, %s,
- _(--roots and --tree are mutually exlusive));
+ _(--roots and --tree are mutually exclusive));
return false;
}
+if (from) {
+vshError(ctl, %s,
+ _(--roots and --from are mutually exclusive));
+}
flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
}
@@ -13161,16 +13174,29 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (dom == NULL)
goto cleanup;
-numsnaps = virDomainSnapshotNum(dom, flags);
-
-/* Fall back to simulation if --roots was unsupported. */
-if (numsnaps 0 last_error-code == VIR_ERR_INVALID_ARG
-(flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
-virFreeError(last_error);
-last_error = NULL;
-parent_filter = -1;
-flags = ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+if (from) {
+start = virDomainSnapshotLookupByName(dom, from, 0);
+if (!start)
+goto cleanup;
+if (vshCommandOptBool(cmd, descendants) || tree) {
+flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+}
+numsnaps = virDomainSnapshotNumChildren(start, flags);
+if (numsnaps = 0 tree)
+numsnaps++;
+/* XXX Is it worth emulating --from on older servers? */
+} else {
numsnaps = virDomainSnapshotNum(dom, flags);
+
+/* Fall back to simulation if --roots was unsupported. */
+if (numsnaps 0 last_error-code == VIR_ERR_INVALID_ARG
+(flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
+virFreeError(last_error);
+last_error = NULL;
+parent_filter = -1;
+flags = ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+numsnaps = virDomainSnapshotNum(dom, flags);
+}
}
if (numsnaps 0)
@@ -13196,14 +13222,32 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (VIR_ALLOC_N(names, numsnaps) 0)
goto cleanup;
-actual = virDomainSnapshotListNames(dom, names, numsnaps, flags);
+if (from) {
+/* When mixing --from and --tree, we want to start the tree at the
+ * given snapshot. Without --tree, only list the children. */
+if (tree) {
+if (numsnaps)
+actual = virDomainSnapshotListChildrenNames(start, names + 1,
+numsnaps - 1,
+flags);
+if (actual = 0) {
+actual++;
+names[0] = vshStrdup(ctl, from);
+}
+} else {
+actual = virDomainSnapshotListChildrenNames(start, names,
+numsnaps, flags);
[libvirt] [PATCHv2 6/7] snapshot: remote protocol for snapshot children
Very mechanical. I'm so glad we've automated the generation of things,
compared to what it was in 0.8.x days, where this would be much longer.
* src/remote/remote_protocol.x
(REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN)
(REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES): New rpcs.
(remote_domain_snapshot_num_children_args)
(remote_domain_snapshot_num_children_ret)
(remote_domain_snapshot_list_children_names_args)
(remote_domain_snapshot_list_children_names_ret): New structs.
* src/remote/remote_driver.c (remote_driver): Use it.
* src/remote_protocol-structs: Update.
---
v2: fix typo in remote_protocol-structs
src/remote/remote_driver.c |2 ++
src/remote/remote_protocol.x | 25 +++--
src/remote_protocol-structs | 20
3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 83f4f3c..0e303df 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -4411,6 +4411,8 @@ static virDriver remote_driver = {
.domainSnapshotGetXMLDesc = remoteDomainSnapshotGetXMLDesc, /* 0.8.0 */
.domainSnapshotNum = remoteDomainSnapshotNum, /* 0.8.0 */
.domainSnapshotListNames = remoteDomainSnapshotListNames, /* 0.8.0 */
+.domainSnapshotNumChildren = remoteDomainSnapshotNumChildren, /* 0.9.7 */
+.domainSnapshotListChildrenNames = remoteDomainSnapshotListChildrenNames,
/* 0.9.7 */
.domainSnapshotLookupByName = remoteDomainSnapshotLookupByName, /* 0.8.0 */
.domainHasCurrentSnapshot = remoteDomainHasCurrentSnapshot, /* 0.8.0 */
.domainSnapshotGetParent = remoteDomainSnapshotGetParent, /* 0.9.7 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index c8a92fd..f95253e 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2067,6 +2067,25 @@ struct remote_domain_snapshot_list_names_ret {
remote_nonnull_string namesREMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX; /*
insert@1 */
};
+struct remote_domain_snapshot_num_children_args {
+remote_nonnull_domain_snapshot snap;
+unsigned int flags;
+};
+
+struct remote_domain_snapshot_num_children_ret {
+int num;
+};
+
+struct remote_domain_snapshot_list_children_names_args {
+remote_nonnull_domain_snapshot snap;
+int maxnames;
+unsigned int flags;
+};
+
+struct remote_domain_snapshot_list_children_names_ret {
+remote_nonnull_string namesREMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX; /*
insert@1 */
+};
+
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
@@ -2524,8 +2543,10 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB = 241, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_SPEED = 242, /* autogen autogen */
REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243, /* skipgen skipgen */
-REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen */
-REMOTE_PROC_DOMAIN_RESET = 245 /* autogen autogen */
+REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen
priority:high */
+REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */
+REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen
priority:high */
+REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247 /* autogen autogen
priority:high */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 69175cc..7894441 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1557,6 +1557,24 @@ struct remote_domain_snapshot_list_names_ret {
remote_nonnull_string * names_val;
} names;
};
+struct remote_domain_snapshot_num_children_args {
+remote_nonnull_domain_snapshot snap;
+u_int flags;
+};
+struct remote_domain_snapshot_num_children_ret {
+intnum;
+};
+struct remote_domain_snapshot_list_children_names_args {
+remote_nonnull_domain_snapshot snap;
+intmaxnames;
+u_int flags;
+};
+struct remote_domain_snapshot_list_children_names_ret {
+struct {
+u_int names_len;
+remote_nonnull_string * names_val;
+} names;
+};
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
@@ -1971,4 +1989,6 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_BLOCK_STATS_FLAGS = 243,
REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244,
REMOTE_PROC_DOMAIN_RESET = 245,
+REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246,
+REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
};
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 7/7] snapshot: implement snapshot children listing in qemu
Not too hard to wire up. The trickiest part is realizing that
listing children of a snapshot cannot use SNAPSHOT_LIST_ROOTS,
and that we overloaded that bit to also mean SNAPSHOT_LIST_DESCENDANTS;
we use that bit to decide which iteration to use, but don't want
the existing counting/listing functions to see that bit.
* src/conf/domain_conf.h (virDomainSnapshotObjListNumFrom)
(virDomainSnapshotObjListGetNamesFrom): New prototypes.
* src/conf/domain_conf.c (virDomainSnapshotObjListNumFrom)
(virDomainSnapshotObjListGetNamesFrom): New functions.
* src/libvirt_private.syms (domain_conf.h): Export them.
* src/qemu/qemu_driver.c (qemuDomainSnapshotNumChildren)
(qemuDomainSnapshotListChildrenNames): New functions.
---
v2: no change, but now virsh changes have been tested both with
and without this patch
src/conf/domain_conf.c | 51 +++
src/conf/domain_conf.h |7
src/libvirt_private.syms |2 +
src/qemu/qemu_driver.c | 87 ++
4 files changed, 147 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c141982..438e3b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12138,6 +12138,37 @@ cleanup:
return -1;
}
+int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
+ virDomainSnapshotObjListPtr snapshots,
+ char **const names, int maxnames,
+ unsigned int flags)
+{
+struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
+int i;
+
+data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+
+if (flags VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
+virDomainSnapshotForEachDescendant(snapshots, snapshot,
+ virDomainSnapshotObjListCopyNames,
+ data);
+else
+virDomainSnapshotForEachChild(snapshots, snapshot,
+ virDomainSnapshotObjListCopyNames,
data);
+
+if (data.oom) {
+virReportOOMError();
+goto cleanup;
+}
+
+return data.numnames;
+
+cleanup:
+for (i = 0; i data.numnames; i++)
+VIR_FREE(data.names[i]);
+return -1;
+}
+
struct virDomainSnapshotNumData {
int count;
unsigned int flags;
@@ -12165,6 +12196,26 @@ int
virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
return data.count;
}
+int
+virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
+virDomainSnapshotObjListPtr snapshots,
+unsigned int flags)
+{
+struct virDomainSnapshotNumData data = { 0, 0 };
+
+data.flags = flags ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
+
+if (flags VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
+virDomainSnapshotForEachDescendant(snapshots, snapshot,
+ virDomainSnapshotObjListCount,
+ data);
+else
+virDomainSnapshotForEachChild(snapshots, snapshot,
+ virDomainSnapshotObjListCount, data);
+
+return data.count;
+}
+
virDomainSnapshotObjPtr
virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots,
const char *name)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0bc0042..1258740 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1488,6 +1488,13 @@ int
virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
unsigned int flags);
int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
unsigned int flags);
+int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
+ virDomainSnapshotObjListPtr snapshots,
+ char **const names, int maxnames,
+ unsigned int flags);
+int virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
+virDomainSnapshotObjListPtr snapshots,
+unsigned int flags);
virDomainSnapshotObjPtr virDomainSnapshotFindByName(const
virDomainSnapshotObjListPtr snapshots,
const char *name);
void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c2a3fab..6b9ceaa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -413,7 +413,9 @@ virDomainSnapshotForEachChild;
virDomainSnapshotForEachDescendant;
virDomainSnapshotHasChildren;
virDomainSnapshotObjListGetNames;
+virDomainSnapshotObjListGetNamesFrom;
[libvirt] [PATCHv2 5/7] snapshot: virsh fallback for snapshot-list --descendants --from
Given a list of snapshots and their parents, finding all descendants
requires a hairy traversal. This code is O(n^3); it could maybe be
made to scale O(n^2) with the use of a hash table, but that costs more
memory. Hopefully there aren't too many people with a hierarchy
so large as to approach REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX (1024).
* tools/virsh.c (cmdSnapshotList): Add final fallback.
---
v2: rebase around ctl flag
tools/virsh.c | 67 +++--
1 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index b2523d3..ec6f854 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13133,6 +13133,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
bool tree = vshCommandOptBool(cmd, tree);
const char *from = NULL;
virDomainSnapshotPtr start = NULL;
+int start_index = -1;
bool descendants = false;
if (vshCommandOptString(cmd, from, from) 0) {
@@ -13187,10 +13188,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
numsnaps = ctl-useSnapshotOld ? -1 :
virDomainSnapshotNumChildren(start, flags);
if (numsnaps 0) {
-/* XXX also want to emulate --descendants without --tree */
-if ((!descendants || tree)
-(ctl-useSnapshotOld ||
- last_error-code == VIR_ERR_NO_SUPPORT)) {
+if (ctl-useSnapshotOld ||
+last_error-code == VIR_ERR_NO_SUPPORT) {
/* We can emulate --from. */
virFreeError(last_error);
last_error = NULL;
@@ -13269,6 +13268,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (tree || ctl-useSnapshotOld) {
parents = vshCalloc(ctl, sizeof(char *), actual);
for (i = (from !ctl-useSnapshotOld); i actual; i++) {
+if (ctl-useSnapshotOld STREQ(names[i], from)) {
+start_index = i;
+continue;
+}
+
/* free up memory from previous iterations of the loop */
if (snapshot)
virDomainSnapshotFree(snapshot);
@@ -13302,12 +13306,61 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
} else {
if (ctl-useSnapshotOld descendants) {
-/* XXX emulate --descendants as well */
-goto cleanup;
+bool changed = false;
+
+/* Make multiple passes over the list - first pass NULLs
+ * out all roots except start, remaining passes NULL out
+ * any entry whose parent is not still in list. Also, we
+ * NULL out parent when name is known to be in list.
+ * Sorry, this is O(n^3) - hope your hierarchy isn't huge. */
+if (start_index 0) {
+vshError(ctl, _(snapshot %s disappeared from list), from);
+goto cleanup;
+}
+for (i = 0; i actual; i++) {
+if (i == start_index)
+continue;
+if (!parents[i]) {
+VIR_FREE(names[i]);
+} else if (STREQ(parents[i], from)) {
+VIR_FREE(parents[i]);
+changed = true;
+}
+}
+if (!changed) {
+ret = true;
+goto cleanup;
+}
+while (changed) {
+changed = false;
+for (i = 0; i actual; i++) {
+bool found = false;
+int j;
+
+if (!names[i] || !parents[i])
+continue;
+for (j = 0; j actual; j++) {
+if (!names[j] || i == j)
+continue;
+if (STREQ(parents[i], names[j])) {
+found = true;
+if (!parents[j])
+VIR_FREE(parents[i]);
+break;
+}
+}
+if (!found) {
+changed = true;
+VIR_FREE(names[i]);
+}
+}
+}
+VIR_FREE(names[start_index]);
}
for (i = 0; i actual; i++) {
-if (ctl-useSnapshotOld STRNEQ_NULLABLE(parents[i], from))
+if (ctl-useSnapshotOld
+(descendants ? !names[i] : STRNEQ_NULLABLE(parents[i], from)))
continue;
/* free up memory from previous iterations of the loop */
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: do not unlink NULL file
error:could not take a screenshot of xp
==6216== Syscall param unlink(pathname) points to unaddressable byte(s)
==6216==at 0x373A0D4937: unlink (syscall-template.S:82)
==6216==by 0x40FD73: cmdScreenshot (virsh.c:3070)
==6216==by 0x42BA0D: vshCommandRun (virsh.c:14920)
==6216==by 0x42EC97: main (virsh.c:16379)
==6216== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==6216==
error:Requested operation is not valid: domain is not running
---
tools/virsh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 1909dce..89d355f 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3004,7 +3004,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
unsigned int screen = 0;
unsigned int flags = 0; /* currently unused */
int ret = false;
-bool created = true;
+bool created = false;
bool generated = false;
char *mime = NULL;
@@ -3039,13 +3039,13 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
}
if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) 0) {
-created = false;
if (errno != EEXIST ||
(fd = open(file, O_WRONLY|O_TRUNC, 0666)) 0) {
vshError(ctl, _(cannot create file %s), file);
goto cleanup;
}
-}
+} else
+created = true;
if (virStreamRecvAll(st, vshStreamSink, fd) 0) {
vshError(ctl, _(could not receive data from domain %s), name);
--
1.7.6.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: do not unlink NULL file
On 09/30/2011 07:05 PM, Marc-André Lureau wrote:
error:could not take a screenshot of xp
==6216== Syscall param unlink(pathname) points to unaddressable byte(s)
==6216==at 0x373A0D4937: unlink (syscall-template.S:82)
==6216==by 0x40FD73: cmdScreenshot (virsh.c:3070)
==6216==by 0x42BA0D: vshCommandRun (virsh.c:14920)
==6216==by 0x42EC97: main (virsh.c:16379)
==6216== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==6216==
error:Requested operation is not valid: domain is not running
---
tools/virsh.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
ACK.
-}
+} else
+created = true;
Style. Any if-else that requires {} on one arm should use {} on both
arms. I fixed that and 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
[libvirt] [PATCH] Set to NULL members that have been freed to prevent crashes
Do not crash if virStreamFinish is called after error.
==11000== Invalid read of size 4
==11000==at 0x373A8099A0: pthread_mutex_lock (pthread_mutex_lock.c:51)
==11000==by 0x4C7CADE: virMutexLock (threads-pthread.c:85)
==11000==by 0x4D57C31: virNetClientStreamRaiseError
(virnetclientstream.c:203)
==11000==by 0x4D385E4: remoteStreamFinish (remote_driver.c:3541)
==11000==by 0x4D182F9: virStreamFinish (libvirt.c:14157)
==11000==by 0x40FDC4: cmdScreenshot (virsh.c:3075)
==11000==by 0x42BA40: vshCommandRun (virsh.c:14922)
==11000==by 0x42ECCA: main (virsh.c:16381)
==11000== Address 0x59b86c0 is 16 bytes inside a block of size 216 free'd
==11000==at 0x4A06928: free (vg_replace_malloc.c:427)
==11000==by 0x4C69E2B: virFree (memory.c:310)
==11000==by 0x4D57B56: virNetClientStreamFree (virnetclientstream.c:184)
==11000==by 0x4D3DB7A: remoteDomainScreenshot (remote_client_bodies.h:1812)
==11000==by 0x4CFD245: virDomainScreenshot (libvirt.c:2903)
==11000==by 0x40FB73: cmdScreenshot (virsh.c:3029)
==11000==by 0x42BA40: vshCommandRun (virsh.c:14922)
==11000==by 0x42ECCA: main (virsh.c:16381)
---
src/rpc/gendispatch.pl |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 039d785..b7ac3c8 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1480,6 +1480,8 @@ elsif ($opt_k) {
if ($call-{streamflag} ne none) {
print virNetClientRemoveStream(priv-client, netst);\n;
print virNetClientStreamFree(netst);\n;
+print st-driver = NULL;\n;
+print st-privateData = NULL;\n;
}
print goto done;\n;
--
1.7.6.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
