[libvirt] [test-API][PATCH] Add test case of set vcpus with flags
Use setVcpusFlags API to set domain vcpu with flags, domain could be active or not. Flags could be '0', 'live', 'config', 'maximum' and their combinations, use '|' between flags for combinations. A sample conf file also added. Signed-off-by: Wayne Sun g...@redhat.com --- cases/set_vcpus_flags.conf | 56 repos/domain/set_vcpus_flags.py | 283 +++ 2 files changed, 339 insertions(+), 0 deletions(-) create mode 100644 cases/set_vcpus_flags.conf create mode 100644 repos/domain/set_vcpus_flags.py diff --git a/cases/set_vcpus_flags.conf b/cases/set_vcpus_flags.conf new file mode 100644 index 000..7da13a2 --- /dev/null +++ b/cases/set_vcpus_flags.conf @@ -0,0 +1,56 @@ +domain:install_linux_cdrom +guestname +$defaultname +guestos +$defaultos +guestarch +$defaultarch +vcpu +$defaultvcpu +memory +$defaultmem +hddriver +$defaulthd +nicdriver +$defaultnic +imageformat +qcow2 + +domain:set_vcpus_flags +guestname +$defaultname +vcpu +4 +flags +0|config|live +username +$username +password +$password + +domain:destroy +guestname +$defaultname + +domain:set_vcpus_flags +guestname +$defaultname +vcpu +5 +flags +0|config + +domain:set_vcpus_flags +guestname +$defaultname +vcpu +6 +flags +maximum + +domain:undefine +guestname +$defaultname + +options cleanup=enable + diff --git a/repos/domain/set_vcpus_flags.py b/repos/domain/set_vcpus_flags.py new file mode 100644 index 000..ca614cc --- /dev/null +++ b/repos/domain/set_vcpus_flags.py @@ -0,0 +1,283 @@ +#!/usr/bin/env python +# Test set domain vcpu with flags. Flags could be 0, live, config, +# maximum and their combinations, use '|' for combinations. If +# domain is active, username and password should be provided, else +# not. + +import time +import commands +from xml.dom import minidom + +import libvirt +from libvirt import libvirtError + +from src import sharedmod +from utils import utils + +required_params = ('guestname', 'vcpu', 'flags', ) +optional_params = { + 'username': 'root', + 'password': '', + } + +def check_domain_running(conn, guestname): + check if the domain exists, may or may not be active +guest_names = [] +ids = conn.listDomainsID() +for id in ids: +obj = conn.lookupByID(id) +guest_names.append(obj.name()) + +if guestname not in guest_names: +logger.info(%s is not running % guestname) +return 1 +else: +return 0 + +def redefine_vcpu_number(domobj, guestname, vcpu): +dump domain xml description to change the vcpu number, + then, define the domain again + +guestxml = domobj.XMLDesc(0) +logger.debug('''original guest %s xml :\n%s''' %(guestname, guestxml)) + +doc = minidom.parseString(guestxml) + +newvcpu = doc.createElement('vcpu') +newvcpuval = doc.createTextNode(str(vcpu)) +newvcpu.appendChild(newvcpuval) +newvcpu.setAttribute('current', '1') + +domain = doc.getElementsByTagName('domain')[0] +oldvcpu = doc.getElementsByTagName('vcpu')[0] + +domain.replaceChild(newvcpu, oldvcpu) + +return doc.toxml() + +def check_current_vcpu(domobj, state, flag, username, password): +dump domain xml description to get current vcpu number and + check vcpu in domain if domain is active + +if len(flag) == 1 and flag[0] == '0': +if state: +flag.append('config') +else: +flag.append('live') + +if len(flag) == 1 and flag[0] == 'maximum': +if state: +flag.append('config') +else: +logger.error('maximum' on live domain is not supported') +return False + +if 'live' in flag: +if 'maximum' in flag: +logger.error('live' with 'maximum' is not supported') +return False + +guestxml = domobj.XMLDesc(1) +logger.debug(domain %s xml is :\n%s %(domobj.name(), guestxml)) + +xml = minidom.parseString(guestxml) +vcpu = xml.getElementsByTagName('vcpu')[0] +if vcpu.hasAttribute('current'): +attr = vcpu.getAttributeNode('current') +current_vcpu = int(attr.nodeValue) +else: +logger.info(no 'current' attribute in vcpu element) +return False + +if not state: +if password == '' and username == 'root': +logger.error(check will fail with empty root password) +return False + +logger.info(check cpu number in domain) +ip = utils.mac_to_ip(mac, 180) + +cmd = cat /proc/cpuinfo | grep processor | wc -l +ret, output = utils.remote_exec_pexpect(ip, username, password, cmd) +if
Re: [libvirt] RFC: Migration with NPIV
On Mon, Nov 19, 2012 at 06:42:42PM +0800, Osier Yang wrote: One API missed is: int virNodeDeviceCreate(virNodeDevicePtr dev, unsigned int flags); To create the vHBA. That API + functionality already exists 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: Migration with NPIV
On Mon, Nov 19, 2012 at 05:30:11PM +0800, Osier Yang wrote: Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce autostart for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); I don't really much like this approach. IMHO, this should all be done via the virStoragePool APIs instead. Adding define/undefine/autostart to virNodeDevice is really just duplicating the storage pool functionality. 2) Associate vHBA with domain XML There are two ways to attach a LUN to a domain: as an QEMU emulated device; or passthrough. Since passthrough a LUN is not supported in libvirt yet, let's focus on the emulated LUN at this stage. New attributes wwnn and wwpn are introduced to indicate the LUN behind the vHBA. E.g. disk type='block' device='disk' driver name='qemu' type='raw'/ source wwnn=2001001b32a9da4e wwpn=2101001b32a90004/ If you change the schema of the source element, then you must also create a new type='XXX' attribute to identify it, not just re-use type='block' target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /disk Before the domain starting, we have to check if there is LUN assigned to the vHBA, error out if not. Using the stable path of LUN also works, e.g. source dev=/dev/disk/by-path/pci-\:00\:07.0-scsi-0\:0\:0\:0/ But the disadvantage is the user have to figure out the stable path himself; And we have to do checking of every stable path to see if it's behind a vHBA in migration Begin stage. Or an new XML tag for element source to indicate that it's behind a vHBA? such as: source dev=disk-by-path model=vport/ I don't much like the idea of mapping vHBA to disk elements, because you have a cardinality mis-match. A disk is equivalent of a single LUN, but a vHBA is something that provides multiple LUNs. If you want to directly associate a vHBA with a virtual guest, then this is really in the realm of SCSI HBA passthrough, not disk devices. If you want something mapped to the disk device, then the approach should be to map to a storage pool volume - something we've long talked about as broadly useful for all storage types, not just NPIV. 3) Migration with vHBA One possible solution for migration with vHBA is to use one pair of WWNN WWPN on source host, one is using for domain, one is reserved for migration purpose. It requires the storage admin maps the same LUN to the two vHBAs when doing the masking and zoning. One of the two vHBA is called Primary vHBA, another is called secondary vHBA. To maitain the relationship between these two vHBAs, we have to introduce new XMLs to vHBA. E.g. In XML of primary vHBA: secondary wwpn=2101001b32a90004/ In XML of secondary vHBA: primary wwpn=2101001b32a90002/ Primary vHBA is going to be guaranteed not used by any domain which is driven by libvirt (we do some checking eariler before the domain starting). And it's also guaranteed that the LUN can't be used by other domain with sVirt or Sanlock. So it's safe to have two vHBAs on source host too. To prevent one using the LUN by creating vHBA using the same WWNN WWPN on another host, we must create the secondary vHBA on source host, even it's not being used. Both primary and secondary vHBA must be defined and marked
Re: [libvirt] Report guest IPs as XML or struct?
Seems like we agree on struct. So this is my proposal: typedef enum { VIR_INTERFACE_TYPE_UNKNOWN = 0, VIR_INTERFACE_TYPE_ETHER = 1, VIR_INTERFACE_TYPE_PPP = 2, /* we can add things here */ #idef VIR_ENUM_SENTINELS VIR_INTERFACE_TYPE_LAST #endif } virDomainInterfaceType; typedef enum { VIR_IP_ADDR_TYPE_IPV4, VIR_IP_ADDR_TYPE_IPV6, #ifdef VIR_ENUM_SENTINELS VIR_IP_ADDR_TYPE_LAST #endif } virIPAddrType; typedef struct _virDomainInterfaceIPAddress virDomainIPAddress; typedef virDomainIPAddress *virDomainIPAddressPtr; struct _virDomainInterfaceIPAddress { int type; /* virIPAddrType */ char *addr; /* IP address */ int prefix; /* IP address prefix */ }; typedef struct _virDomainInterface virDomainInterface; typedef virDomainInterface *virDomainInterfacePtr; struct _virDomainInterface { char *name; /* interface name */ int type; /* virDomainInterfaceType */ char *hwaddr; /* hardware address */ unsigned int ip_addrs_count;/* number of items in @ip_addr */ virDomainIPAddressPtr ip_addrs; /* array of IP addresses */ void *padding[20]; /* maybe we need this? */ }; typedef enum { VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT = 0, /* hypervisor choice */ VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT = 1, /* use guest agent */ VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER = 2, /* use nwfilter learning code */ /* etc ... */ } virDomainInterfacesAddressesMethod; int virDomainInterfacesAddresses(virDomainPtr dom, virDomainInterfacePtr ifaces, unsigned int method unsigned int flags); This API would dynamically allocate return array for all interfaces presented in @dom. We are already doing this in some places, so I think it's okay. With @method users can choose desired way to get info. I guess if we misuse @flags for that it would be just a waste of space, since methods are mutually exclusive. Hence, @flags are currently unused. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Report sensible error for invalid disk name
The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. --- src/util/util.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/util.c b/src/util/util.c index 75b18c1..d5b2c97 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) { } } -if (!ptr) +if (!ptr) { +virReportError(VIR_ERR_XML_ERROR, + _(Unknown disk name '%s' and no address specified), + name); return -1; +} for (i = 0; *ptr; i++) { idx = (idx + (i 1 ? 0 : 1)) * 26; -- 1.8.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Report sensible error for invalid disk name
On 11/20/12 14:55, Martin Kletzander wrote: The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. --- src/util/util.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/util.c b/src/util/util.c index 75b18c1..d5b2c97 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) { } } -if (!ptr) +if (!ptr) { +virReportError(VIR_ERR_XML_ERROR, + _(Unknown disk name '%s' and no address specified), + name); return -1; +} Some of the call paths of this function the callers do their own error reporting (see src/qemu/qemu_command.c) based on the return value. This should be consolidated. In case of this function I'd probably rather go for adding the error message on the appropriate places as the inability to parse the disk ID is ignored on some calls (maybe that should be fixed in the first place and than the error reporting is okay here.) for (i = 0; *ptr; i++) { idx = (idx + (i 1 ? 0 : 1)) * 26; Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Report sensible error for invalid disk name
On Tue, Nov 20, 2012 at 02:55:28PM +0100, Martin Kletzander wrote: The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. --- src/util/util.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/util.c b/src/util/util.c index 75b18c1..d5b2c97 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) { } } -if (!ptr) +if (!ptr) { +virReportError(VIR_ERR_XML_ERROR, + _(Unknown disk name '%s' and no address specified), + name); return -1; +} for (i = 0; *ptr; i++) { idx = (idx + (i 1 ? 0 : 1)) * 26; IMHO, you should really be adding an ATTRIBUTE_NONNULL annotation and then fixing the callers not to invoke it if the disk name they have is Null. Adding virReportError here is wrong, because a number of callers will already report errors. 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] [PATCH] conf: Report sensible error for invalid disk name
On 11/20/2012 03:27 PM, Peter Krempa wrote: On 11/20/12 14:55, Martin Kletzander wrote: The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. --- src/util/util.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/util.c b/src/util/util.c index 75b18c1..d5b2c97 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) { } } -if (!ptr) +if (!ptr) { +virReportError(VIR_ERR_XML_ERROR, + _(Unknown disk name '%s' and no address specified), + name); return -1; +} Some of the call paths of this function the callers do their own error reporting (see src/qemu/qemu_command.c) based on the return value. This should be consolidated. In case of this function I'd probably rather go for adding the error message on the appropriate places as the inability to parse the disk ID is ignored on some calls (maybe that should be fixed in the first place and than the error reporting is okay here.) You're right, I went through the code just looking for the un-errored negative return and haven't thought about that deeply enough. I figured when the error gets set on two places, it'll be simply shadowed. Sending a v2 in a minute. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Report sensible error for invalid disk name
On 11/20/2012 03:45 PM, Daniel P. Berrange wrote: On Tue, Nov 20, 2012 at 02:55:28PM +0100, Martin Kletzander wrote: The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. --- src/util/util.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/util.c b/src/util/util.c index 75b18c1..d5b2c97 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) { } } -if (!ptr) +if (!ptr) { +virReportError(VIR_ERR_XML_ERROR, + _(Unknown disk name '%s' and no address specified), + name); return -1; +} for (i = 0; *ptr; i++) { idx = (idx + (i 1 ? 0 : 1)) * 26; IMHO, you should really be adding an ATTRIBUTE_NONNULL annotation and then fixing the callers not to invoke it if the disk name they have is Null. The pointer is not the function attribute, but ... Adding virReportError here is wrong, because a number of callers will already report errors. ... you're right as Peter, v2 is on it's way. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCHv2 3/n] snapshot: make cloning of domain definition easier
On 11/20/12 01:09, Eric Blake wrote: Upcoming patches for revert-and-clone branching of snapshots need to be able to copy a domain definition; make this step reusable. * src/conf/domain_conf.h (virDomainDefCopy): New prototype. * src/conf/domain_conf.c (virDomainObjCopyPersistentDef): Split... (virDomainDefCopy): ...into new function. (virDomainObjSetDefTransient): Use it. * src/libvirt_private.syms (domain_conf.h): Export it. * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use it. --- src/conf/domain_conf.c | 37 +++-- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 14 ++ 4 files changed, 28 insertions(+), 26 deletions(-) ACK and you can push this out of order. I'll fix my series to take this improvement into account. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] conf: Report sensible error for invalid disk name
The error ... but the cause is unknown appeared for XMLs similar to this: disk type='file' device='cdrom' driver name='qemu' type='raw'/ source file='/dev/zero'/ target dev='sr0'/ /disk Notice unsupported disk type (for the driver), but also no address specified. The first part is not a problem and we should not abort immediately because of that, but the combination with the address unknown was causing an unspecified error. While fixing this, I added an error to one place where this return value was not managed properly. --- v2: - Error moved from virDiskNameToIndex@util/util.c to virDomainDiskDefAssignAddress@conf/domain_conf.c - One more error added into qemuParseCommandLine@qemu/qemu_command.c src/conf/domain_conf.c | 6 +- src/qemu/qemu_command.c | 6 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed21f0f..3a1be02 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3063,8 +3063,12 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def-dst); -if (idx 0) +if (idx 0) { +virReportError(VIR_ERR_XML_ERROR, + _(Unknown disk name '%s' and no address specified), + def-dst); return -1; +} switch (def-bus) { case VIR_DOMAIN_DISK_BUS_SCSI: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 22bb209..097de9b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8342,8 +8342,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, !disk-dst) goto no_memory; -if (virDomainDiskDefAssignAddress(caps, disk) 0) +if (virDomainDiskDefAssignAddress(caps, disk) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Cannot assign address for device name '%s'), + disk-dst); goto error; +} if (VIR_REALLOC_N(def-disks, def-ndisks+1) 0) goto no_memory; -- 1.8.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCHv2 3/n] snapshot: make cloning of domain definition easier
On 11/20/2012 08:05 AM, Peter Krempa wrote: On 11/20/12 01:09, Eric Blake wrote: Upcoming patches for revert-and-clone branching of snapshots need to be able to copy a domain definition; make this step reusable. * src/conf/domain_conf.h (virDomainDefCopy): New prototype. * src/conf/domain_conf.c (virDomainObjCopyPersistentDef): Split... (virDomainDefCopy): ...into new function. (virDomainObjSetDefTransient): Use it. * src/libvirt_private.syms (domain_conf.h): Export it. * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use it. --- src/conf/domain_conf.c | 37 +++-- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 14 ++ 4 files changed, 28 insertions(+), 26 deletions(-) ACK and you can push this out of order. I'll fix my series to take this improvement into account. Done. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCHv2 1/n] snapshot: add revert-and-create branching of external snapshots
On 11/20/12 01:09, Eric Blake wrote: Right now, libvirt refuses to revert to the state at the time of an external snapshot, because doing so would reset things so that the next time the domain boots, we are using the backing file; but modifying the backing file invalidates all qcow2 files that are based on top of it. There are three possibilities for lifting this restriction: 1. delete all snapshot metadata and qcow2 files that are invalidated by the revert (losing changes since the snapshot) 2. perform a block commit (such as with qemu-img commit) to merge the qcow2 file back into the backing file (keeping the changes since the snapshot, but losing the qcow2 files) 3. create NEW qcow2 files that wrap the same base file as the original snapshot (keeping the changes since the original snapshot) This patch documents the API for option 3, by adding a new flag to virDomainSnapshotCreateXML (the only snapshot-related function that takes XML, which is required to pass in new file names during the branch), and wires up virsh to expose it. Later patches will enhance virDomainRevertToSnapshot to cover options 1 and 2. API wise, an application wishing to do the revert-and-create operation must add a branchname/branch element to their domainsnapshot XML, and pass VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH in the flags argument. In virsh, snapshot-create gains a new boolean flag --branch that must match the XML passed in, and snapshot-create-as gains a new --branch=name argument along with a --current boolean for creating such XML. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH): New flag. * src/libvirt.c (virDomainSnapshotCreateXML): Document it, and enforce some mutual exclusion. * docs/formatsnapshot.html.in: Document the new branch element. * docs/schemas/domainsnapshot.rng: Allow new element. * tools/virsh-snapshot.c (cmdSnapshotCreate): Add --branch option. (cmdSnapshotCreateAs): Likewise, also add --current. * tools/virsh.pod (snapshot-create, snapshot-create-as): Document new usage. --- docs/formatsnapshot.html.in | 16 +--- docs/schemas/domainsnapshot.rng | 5 + include/libvirt/libvirt.h.in| 3 +++ src/libvirt.c | 27 ++- tools/virsh-snapshot.c | 40 tools/virsh.pod | 16 ++-- 6 files changed, 89 insertions(+), 18 deletions(-) Looks well designed. ACK. Hopefully no changes will be needed :) Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCHv2 2/n] snapshot: prepare to parse new XML
On 11/20/12 01:09, Eric Blake wrote: No semantic change, but prepare for a new mode of parsing where a new _BRANCH flag requests that the parse look up the existing snapshot to branch from. * src/conf/snapshot_conf.h (VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH): New flag, unused for now. (virDomainSnapshotDefParseString): Add parameter. * src/conf/snapshot_conf.c (virDomainSnapshotDefParseString): Update signature. * src/esx/esx_driver.c (esxDomainSnapshotCreateXML): Update caller. * src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotCreateXML): Likewise. * tests/domainsnapshotxml2xmltest.c (testCompareXMLToXMLFiles): Likewise. --- src/conf/snapshot_conf.c | 4 +++- src/conf/snapshot_conf.h | 2 ++ src/esx/esx_driver.c | 2 +- src/qemu/qemu_driver.c| 3 ++- src/vbox/vbox_tmpl.c | 2 +- tests/domainsnapshotxml2xmltest.c | 2 +- 6 files changed, 10 insertions(+), 5 deletions(-) Sane incremental change. Good for reviewing :) ACK Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Migration with NPIV
On Tue, Nov 20, 2012 at 10:17:11AM +, Daniel P. Berrange wrote: On Mon, Nov 19, 2012 at 05:30:11PM +0800, Osier Yang wrote: Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce autostart for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); I don't really much like this approach. IMHO, this should all be done via the virStoragePool APIs instead. Adding define/undefine/autostart to virNodeDevice is really just duplicating the storage pool functionality. I like the idea of making vHBAs persist as part of pools; how do you envision it should work? Extend the scsi pools to take a vHBA descriptor and then instantiating the vHBA as part of starting the pool, or something else? 2) Associate vHBA with domain XML There are two ways to attach a LUN to a domain: as an QEMU emulated device; or passthrough. Since passthrough a LUN is not supported in libvirt yet, let's focus on the emulated LUN at this stage. New attributes wwnn and wwpn are introduced to indicate the LUN behind the vHBA. E.g. disk type='block' device='disk' driver name='qemu' type='raw'/ source wwnn=2001001b32a9da4e wwpn=2101001b32a90004/ If you change the schema of the source element, then you must also create a new type='XXX' attribute to identify it, not just re-use type='block' target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /disk Before the domain starting, we have to check if there is LUN assigned to the vHBA, error out if not. Using the stable path of LUN also works, e.g. source dev=/dev/disk/by-path/pci-\:00\:07.0-scsi-0\:0\:0\:0/ But the disadvantage is the user have to figure out the stable path himself; And we have to do checking of every stable path to see if it's behind a vHBA in migration Begin stage. Or an new XML tag for element source to indicate that it's behind a vHBA? such as: source dev=disk-by-path model=vport/ I don't much like the idea of mapping vHBA to disk elements, because you have a cardinality mis-match. A disk is equivalent of a single LUN, but a vHBA is something that provides multiple LUNs. If you want to directly associate a vHBA with a virtual guest, then this is really in the realm of SCSI HBA passthrough, not disk devices. If you want something mapped to the disk device, then the approach should be to map to a storage pool volume - something we've long talked about as broadly useful for all storage types, not just NPIV. +1, we really should take this as an opportunity to add storage volumes as disk devices. 3) Migration with vHBA One possible solution for migration with vHBA is to use one pair of WWNN WWPN on source host, one is using for domain, one is reserved for migration purpose. It requires the storage admin maps the same LUN to the two vHBAs when doing the masking and zoning. One of the two vHBA is called Primary vHBA, another is called secondary vHBA. To maitain the relationship between these two vHBAs, we have to introduce new XMLs to vHBA. E.g. In XML of primary vHBA: secondary wwpn=2101001b32a90004/ In XML of secondary vHBA: primary wwpn=2101001b32a90002/ Primary vHBA is
Re: [libvirt] RFC: Migration with NPIV
On Tue, Nov 20, 2012 at 11:26:53AM -0500, Dave Allan wrote: On Tue, Nov 20, 2012 at 10:17:11AM +, Daniel P. Berrange wrote: On Mon, Nov 19, 2012 at 05:30:11PM +0800, Osier Yang wrote: Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce autostart for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); I don't really much like this approach. IMHO, this should all be done via the virStoragePool APIs instead. Adding define/undefine/autostart to virNodeDevice is really just duplicating the storage pool functionality. I like the idea of making vHBAs persist as part of pools; how do you envision it should work? Extend the scsi pools to take a vHBA descriptor and then instantiating the vHBA as part of starting the pool, or something else? Yes, pretty much that. Create when you start the pool, delete when you destroy the pool. If we do the mapping of HBAs to guest domains using storage pools, then at a guest level, migration requires zero work. It is simply upto the management app to create the storage pool on the destination host with the same Name + UUID, but with the secondary WWNN/WWPN. The nice thing about this, is that you don't need to hardcode details of a secondary WWNN/WWPN up-front. The management app can just decide on those at the time it performs the migration, so 99% of the time there will only need to be a single vHBA setup on the SAN. During migration the mgmt app can setup a second vHBA for the target host, and once complete, delete the original vHBA entirely. Agreed, although there will of course need to be some degree of up-front coordination between the management app and the SAN administrators to avoid having to involve them to migrate a VM. Yep, this is in fact why I like to push off more of this detail to the mgmt app. Libvirt is unable to talk to the SAN, so its better if the mgmt app had more direct control of the VHBA setup/teardown via the storage APIs, than to do it automagically in virDomainMigrate where the mgmt app cannot synchronize so easily. 4) Enrich HBA's XML It's hard to known the vHBAs created from a HBA with current implementation. One have to dump XML of each (v)HBAs and find out the clue with element parent of vHBAs. It's good to introduce new element for HBA like vports, so that one can easily known what (how many) vHBAs are created from the HBA? And also it's good to have the maximum vports the HBA supports. Except these, other useful information should be exposed too, such as the vendor name, the HBA state, PCI address, etc. The new XMLs should be like: vports num='2' max='64' vport name=scsi_host40 wwpn=2101001b32a90004/ vport name=scsi_host40 wwpn=2101001b32a90005/ /vports online/ vendorQLogic/vendor address type=pci domain=0 bus=0 slot=5 function=0/ online, vendor, address make sense to vHBA too. I'm trying to remember how we modelled the parent/child relationship for SR-IOV PCI cards. NPIV is a very similar concept, so we should ideally seek to model the parent/child relationship in the same manner. Physical function: device namepci__01_00_0/name
Re: [libvirt] [RFC PATCHv2 4/n] snapshot: actually compute branch definition from XML
On 11/20/12 01:09, Eric Blake wrote: When asked to parse a branch snapshot XML definition, we have to piece together the definition of the new snapshot from parts of the branch point, as well as run some sanity checks that the branches are compatible. This patch is rather restrictive in what it allows; depending on effort and future needs, we may be able to relax some of those restrictions and allow more cases. * src/conf/snapshot_conf.c (virDomainSnapshotDefParseString): Honor new flag. Now when 3/n is pushed this and the prepare one will be right together. Although the changes in 2/n are trivial I'd probably rather keep them separate for now. --- src/conf/snapshot_conf.c | 91 ++-- 1 file changed, 89 insertions(+), 2 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 10aa5e5..901705e 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -171,7 +171,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virCapsPtr caps, unsigned int expectedVirtTypes, -virDomainSnapshotObjListPtr snapshots ATTRIBUTE_UNUSED, +virDomainSnapshotObjListPtr snapshots, unsigned int flags) { xmlXPathContextPtr ctxt = NULL; @@ -188,6 +188,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, char *memorySnapshot = NULL; char *memoryFile = NULL; bool offline = !!(flags VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE); +virDomainSnapshotObjPtr other = NULL; I'd also set tmp to NULL here and ... xml = virXMLParseCtxt(NULL, xmlStr, _((domain_snapshot)), ctxt); if (!xml) { @@ -223,7 +224,61 @@ virDomainSnapshotDefParseString(const char *xmlStr, def-description = virXPathString(string(./description), ctxt); -if (flags VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { +if (flags VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH) { +if ((flags (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL)) || +!snapshots) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(unexpected parse request)); +goto cleanup; +} + +/* In order to form a branch, we must find the existing + * snapshot, and it must be external. */ +tmp = virXPathString(string(./branch), ctxt); +if (!tmp) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(use of branch flag requires branch element)); +goto cleanup; +} +other = virDomainSnapshotFindByName(snapshots, tmp); +if (!other) { +virReportError(VIR_ERR_XML_ERROR, _(could not find branch '%s'), + tmp); +VIR_FREE(tmp); move this free statement right to the cleanup section. +goto cleanup; +} + +if (!virDomainSnapshotIsExternal(other)) { +virReportError(VIR_ERR_XML_ERROR, + _(branch '%s' is not an external snapshot), tmp); +VIR_FREE(tmp); +goto cleanup; +} +if (!other-def-dom) { +virReportError(VIR_ERR_XML_ERROR, + _(branch '%s' lacks corresponding domain details), + tmp); +VIR_FREE(tmp); +goto cleanup; +} +VIR_FREE(tmp); + +/* The new definition shares the same parent, state, and + * domain as what it is branching from. */ +def-creationTime = tv.tv_sec; +if (other-def-parent +!(def-parent = strdup(other-def-parent))) { +virReportOOMError(); +goto cleanup; +} +def-state = other-def-state; + +/* Any memory or disk in the XML must be consistent with + * the branch point, and any disk not in the XML will be + * populated to match the branch; checked below. */ + +} else if (flags VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { if (virXPathLongLong(string(./creationTime), ctxt, def-creationTime) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -314,6 +369,24 @@ virDomainSnapshotDefParseString(const char *xmlStr, _(memory state cannot be saved with offline snapshot)); goto cleanup; } +if (other) { +/* XXX It would be nice to allow automatic reuse of existing + * memory files, with bookkeeping that prevents deleting a + * file if some other branch is still using it. But for a + * first implementation, it is easier to enforce that the user + * always matches (disk-only branching to disk-only; + * checkpoint branching to checkpoint and giving us a new name + * which we
[libvirt] [RFC] Porting Qemu driver to FreeBSD
Hi, Recently I have picked up Sean's and Hiren's activities in porting Qemu driver on FreeBSD. At this point I have the basics working. Meaning that these things work: - Basic host commands (version, nodeinfo, etc) - Can perform basic operations on the VM (start, stop, destroy, etc) - Basic bridging (bridge creation/removing, adding/removing interfaces, setting addresses etc) Currently I keep FreeBSD specific info here: https://github.com/novel/fbsd-libvirt/blob/freebsd/README-freebsd There are a number things to be done: - Firewall support. Currently I have just disabled iptables calls and add general ipfw NAT rules for all VMs. This appears to be the most tough part for me at this point for a number of reasons: * FreeBSD has two widely used firewall packages: pf and ipfw. I'm not sure if they play nice together, e.g. if I choose to use ipfw, people that have pf configuration will have problems making libvirt work with their existing setup * Need to design a general schema for injection of the rules so they don't harm existing setup ipfw seems to be more appealing for automatic control via external applications like libvirt, but I'm still not sure how to organise rulesets. - Affinity stuff needs to be implemented - Code needs a lot of clean-up, needs to be refactored to extract common cross-platfrom code, etc - NWFilter stuff needs to be implemented (this is sort of a long term goal) The code is available here: https://github.com/novel/fbsd-libvirt/tree/freebsd Current changes could be observed here: https://github.com/novel/fbsd-libvirt/compare/upstream_master...freebsd I would appreciate your thoughts and feedback on that topic. Thanks Roman Bogorodskiy pgpprNadTy4X3.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Log an audit message with the LXC init pid
From: Daniel P. Berrange berra...@redhat.com Currently the LXC driver logs audit messages when a container is started or stopped. These audit messages, however, contain the PID of the libvirt_lxc supervisor process. To enable sysadmins to correlate with audit messages generated by processes /inside/ the container, we need to include the container init process PID. We can't do this in the main 'start' audit message, since the init PID is not available at that point. Instead we output a completely new audit record, that lists both PIDs. type=VIRT_CONTROL msg=audit(1353433750.071:363): pid=20180 uid=0 auid=501 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='virt=lxc op=init vm=busy uuid=dda7b947-0846-1759-2873-0f375df7d7eb vm-pid=20371 init-pid=20372 exe=/home/berrange/src/virt/libvirt/daemon/.libs/lt-libvirtd hostname=? addr=? terminal=pts/6 res=success' --- src/conf/domain_audit.c | 26 ++ src/conf/domain_audit.h | 3 +++ src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 31 ++- src/lxc/lxc_monitor.c| 23 +++ src/lxc/lxc_monitor.h| 5 + src/lxc/lxc_process.c| 8 src/lxc/lxc_protocol.x | 7 ++- 8 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 0f3924a..34cd92e 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -605,6 +605,32 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) virDomainAuditLifecycle(vm, start, reason, success); } +void +virDomainAuditInit(virDomainObjPtr vm, + unsigned long long initpid) +{ +char uuidstr[VIR_UUID_STRING_BUFLEN]; +char *vmname; +const char *virt; + +virUUIDFormat(vm-def-uuid, uuidstr); + +if (!(vmname = virAuditEncode(vm, vm-def-name))) { +VIR_WARN(OOM while encoding audit message); +return; +} + +if (!(virt = virDomainVirtTypeToString(vm-def-virtType))) { +VIR_WARN(Unexpected virt type %d while encoding audit message, vm-def-virtType); +virt = ?; +} + +VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, true, + virt=%s op=init %s uuid=%s vm-pid=%lld init-pid=%lld, + virt, vmname, uuidstr, (long long)vm-pid, initpid); + +VIR_FREE(vmname); +} void virDomainAuditStop(virDomainObjPtr vm, const char *reason) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index db35d09..94b1198 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -31,6 +31,9 @@ void virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void virDomainAuditInit(virDomainObjPtr vm, +unsigned long long pid) +ATTRIBUTE_NONNULL(1); void virDomainAuditStop(virDomainObjPtr vm, const char *reason) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5a07139..23369e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -258,6 +258,7 @@ virDomainAuditCgroupPath; virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; +virDomainAuditInit; virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4746897..65d117a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -123,6 +123,7 @@ struct _virLXCController { /* Server socket */ virNetServerPtr server; +bool firstClient; virNetServerClientPtr client; virNetServerProgramPtr prog; bool inShutdown; @@ -132,6 +133,8 @@ struct _virLXCController { #include lxc_controller_dispatch.h static void virLXCControllerFree(virLXCControllerPtr ctrl); +static int virLXCControllerEventSendInit(virLXCControllerPtr ctrl, + pid_t initpid); static void virLXCControllerQuitTimer(int timer ATTRIBUTE_UNUSED, void *opaque) { @@ -152,6 +155,7 @@ static virLXCControllerPtr virLXCControllerNew(const char *name) goto no_memory; ctrl-timerShutdown = -1; +ctrl-firstClient = true; if (!(ctrl-name = strdup(name))) goto no_memory; @@ -588,6 +592,11 @@ static void *virLXCControllerClientPrivateNew(virNetServerClientPtr client, virNetServerClientSetCloseHook(client, virLXCControllerClientCloseHook); VIR_DEBUG(Got new client %p, client); ctrl-client = client; + +if (ctrl-initpid ctrl-firstClient) +virLXCControllerEventSendInit(ctrl, ctrl-initpid); +ctrl-firstClient = false; + return dummy; } @@ -1283,8 +1292,10 @@ virLXCControllerEventSend(virLXCControllerPtr ctrl, { virNetMessagePtr msg; -if (!ctrl-client) +if (!ctrl-client) { +VIR_WARN(Dropping event %d becuase
Re: [libvirt] [PATCH] Log an audit message with the LXC init pid
On 11/20/2012 10:52 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently the LXC driver logs audit messages when a container is started or stopped. These audit messages, however, contain the PID of the libvirt_lxc supervisor process. To enable sysadmins to correlate with audit messages generated by processes /inside/ the container, we need to include the container init process PID. We can't do this in the main 'start' audit message, since the init PID is not available at that point. Instead we output a completely new audit record, that lists both PIDs. type=VIRT_CONTROL msg=audit(1353433750.071:363): pid=20180 uid=0 auid=501 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='virt=lxc op=init vm=busy uuid=dda7b947-0846-1759-2873-0f375df7d7eb vm-pid=20371 init-pid=20372 exe=/home/berrange/src/virt/libvirt/daemon/.libs/lt-libvirtd hostname=? addr=? terminal=pts/6 res=success' ACK. src/lxc/lxc_protocol.x | 7 ++- 8 files changed, 102 insertions(+), 2 deletions(-) Hmm, we probably ought to start src/lxc_protocol-structs (similar to all our other RPC files) in order to ensure that we don't break ABI compatibility when updating .x files. But that is a separate patch. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ivshmem server
This mail is meant to get a discussion started. Please keep me on cc for the discussion, as I'm not subscribed to libvir-list. ivshmem is an implementation of an inter-VM communication channel. Support for this has been in qemu since v0.14.0 and libvirt patches have been recently posted[1]. What's still missing is the ivshmem server. The ivshmem server is needed when one would like to use interrupts with ivshmem. The server manages a set of eventfds to send/recv those interrupts. There is currently only one implementation of this server that I'm aware of, which is available from this git repo [2] in the ivshmem-server directory. My suggestion for libvirt is that this code be integrated into libvirt, rather than managed by libvirt, for the following reasons 1. libvirt should keep track of the socket path in order to build ivshmem's command line anyway. 2. the current ivshmem server code is ~300 lines, so it shouldn't be a large integration effort. 3. keeping ivhsmem server separate increases the package management that distributions need to do. afaik, it isn't currently packaged for any distribution. One concern I have with the git repo [2] is that I don't see any license for ivshmem-server. I've cc'ed Cam for his input. Drew [1] http://www.redhat.com/archives/libvir-list/2012-November/msg00731.html [2] git://gitorious.org/nahanni/guest-code.git -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ivshmem server
On Tue, Nov 20, 2012 at 01:16:56PM -0500, Andrew Jones wrote: This mail is meant to get a discussion started. Please keep me on cc for the discussion, as I'm not subscribed to libvir-list. ivshmem is an implementation of an inter-VM communication channel. Support for this has been in qemu since v0.14.0 and libvirt patches have been recently posted[1]. What's still missing is the ivshmem server. The ivshmem server is needed when one would like to use interrupts with ivshmem. The server manages a set of eventfds to send/recv those interrupts. There is currently only one implementation of this server that I'm aware of, which is available from this git repo [2] in the ivshmem-server directory. My suggestion for libvirt is that this code be integrated into libvirt, rather than managed by libvirt, for the following reasons 1. libvirt should keep track of the socket path in order to build ivshmem's command line anyway. 2. the current ivshmem server code is ~300 lines, so it shouldn't be a large integration effort. 3. keeping ivhsmem server separate increases the package management that distributions need to do. afaik, it isn't currently packaged for any distribution. One concern I have with the git repo [2] is that I don't see any license for ivshmem-server. I've cc'ed Cam for his input. I guess my main question would be how much more, if any, is the ivshmem-server expected to grow over time ? I wouldn't want to get into a fork-situation if people are planning to do much more dev work on the current ivshmem-server code. NB, if the code is integrated into libvirt, I think it would still have to run in a separate daemon. The reason is that we want to ensure that guests remain fully functional, even when libvirtd is stopped (whether for RPM upgrade, or due to crash) I've no particular strong opinion either way on your proposal. On the surface it seems reasonable to me. 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 PATCHv2 4/n] snapshot: actually compute branch definition from XML
On 11/20/2012 09:41 AM, Peter Krempa wrote: @@ -188,6 +188,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, char *memorySnapshot = NULL; char *memoryFile = NULL; bool offline = !!(flags VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE); +virDomainSnapshotObjPtr other = NULL; I'd also set tmp to NULL here and ... xml = virXMLParseCtxt(NULL, xmlStr, _((domain_snapshot)), ctxt); if (!xml) { @@ -223,7 +224,61 @@ virDomainSnapshotDefParseString(const char *xmlStr, +tmp = virXPathString(string(./branch), ctxt); +if (!tmp) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(use of branch flag requires branch element)); +goto cleanup; +} +other = virDomainSnapshotFindByName(snapshots, tmp); +if (!other) { +virReportError(VIR_ERR_XML_ERROR, _(could not find branch '%s'), + tmp); +VIR_FREE(tmp); move this free statement right to the cleanup section. Good idea. Also, it will let me raise an error if the user passed branch in the xml but did not pass the right flag. +if (other) { +/* XXX It would be nice to allow automatic reuse of existing + * memory files, with bookkeeping that prevents deleting a + * file if some other branch is still using it. But for a + * first implementation, it is easier to enforce that the user + * always matches (disk-only branching to disk-only; + * checkpoint branching to checkpoint and giving us a new name + * which we set up as a copy). There is also a question of + * whether attempting a hard link rather than always copying + * to a new inode makes sense, if the original is a regular + * file and not a block device. */ Hm, despite this was my idea it's looking more strange the more I think about it. If we're going to have the users to specify a new image file now we will need to support that after we come up with a better version. I'm going to think about this a bit more. But for now it seems to be the laziest solution. Actually, now that I'm diving into qemu implementation, it is actually harder to allow (or even require) the user to pass the external file name; in order to link() or copy the original file into the new name, the qemu driver has to know what the old name was. So I either have to alter this function signature to return one more piece of information, or else explicitly plan that any code we do for deleting a snapshot is careful to not delete the external memory file if the snapshot has siblings. Also, while link() is fast, copying is potentially slow (and since state files can be several hundred megabytes, doing a copy can also potentially wreak havoc on the file system cache unless we use O_DIRECT, and I really don't want to go there). So in my next revision of the series, I'm going with the shared file approach, as it just seems cleaner all around. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] qemu: add boot order support for redirected and host USB devices
Commit a4c19459aa8634c43b51e8138fb1d7eec4c17824 missed the functionality for redirected USB devices (fixed in 1/4), documentation and tests. https://bugzilla.redhat.com/show_bug.cgi?id=805414 Ján Tomko (4): conf: add support for booting from redirected USB devices docs: boot order for host and redirected USB devices tests: add boot order for host and redirected USB to qemu argv test tests: update qemuhelptest data docs/formatdomain.html.in | 27 ++- src/conf/domain_conf.c | 34 ++-- tests/qemuhelpdata/qemu-1.2.0 |1 + tests/qemuhelpdata/qemu-1.2.0-device | 51 +++- tests/qemuhelpdata/qemu-kvm-1.2.0 | 276 tests/qemuhelpdata/qemu-kvm-1.2.0-device | 222 tests/qemuhelptest.c | 106 - ...muxml2argv-hostdev-usb-address-device-boot.args |6 + ...emuxml2argv-hostdev-usb-address-device-boot.xml | 28 ++ .../qemuxml2argv-usb-redir-boot.args | 10 + .../qemuxml2argv-usb-redir-boot.xml| 42 +++ tests/qemuxml2argvtest.c |9 + 12 files changed, 775 insertions(+), 37 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-kvm-1.2.0 create mode 100644 tests/qemuhelpdata/qemu-kvm-1.2.0-device create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.xml -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] conf: add support for booting from redirected USB devices
Commit a4c19459aa8634c43b51e8138fb1d7eec4c17824 only added the QEMU capability flag, command line option and added the boot element for redirdev's in the XML schema. This patch adds support for parsing and writing the XML with redirdevs with the boot flag. It also ignores unknown XML elements in redirdev instead of failing with: error: An error occurred, but the cause is unknown Bug: https://bugzilla.redhat.com/show_bug.cgi?id=805414 --- src/conf/domain_conf.c | 34 +- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index be76c06..047c4fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7312,11 +7312,13 @@ error: static virDomainRedirdevDefPtr virDomainRedirdevDefParseXML(const xmlNodePtr node, + virBitmapPtr bootMap, unsigned int flags) { xmlNodePtr cur; virDomainRedirdevDefPtr def; char *bus, *type = NULL; +int remaining; if (VIR_ALLOC(def) 0) { virReportOOMError(); @@ -7348,25 +7350,20 @@ virDomainRedirdevDefParseXML(const xmlNodePtr node, } cur = node-children; -while (cur != NULL) { -if (cur-type == XML_ELEMENT_NODE) { -if (xmlStrEqual(cur-name, BAD_CAST source)) { -int remaining; - -remaining = virDomainChrSourceDefParseXML(def-source.chr, cur, flags, - NULL, NULL, NULL, 0); -if (remaining != 0) -goto error; -} -} -cur = cur-next; -} +/* boot gets parsed in virDomainDeviceInfoParseXML + * source gets parsed in virDomainChrSourceDefParseXML + * we don't know any of the elements that might remain */ +remaining = virDomainChrSourceDefParseXML(def-source.chr, cur, flags, + NULL, NULL, NULL, 0); +if (remaining 0) +goto error; if (def-source.chr.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { def-source.chr.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_USBREDIR; } -if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0) +if (virDomainDeviceInfoParseXML(node, bootMap, def-info, +flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) 0) goto error; if (def-bus == VIR_DOMAIN_REDIRDEV_BUS_USB @@ -7711,7 +7708,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node-name, BAD_CAST redirdev)) { dev-type = VIR_DOMAIN_DEVICE_REDIRDEV; -if (!(dev-data.redirdev = virDomainRedirdevDefParseXML(node, flags))) +if (!(dev-data.redirdev = virDomainRedirdevDefParseXML(node, NULL, flags))) goto error; } else { virReportError(VIR_ERR_XML_ERROR, @@ -8295,7 +8292,8 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, if (virXPathULong(count(./devices/disk[boot] |./devices/interface[boot] - |./devices/hostdev[boot]), ctxt, deviceBoot) 0) { + |./devices/hostdev[boot] + |./devices/redirdev[boot]), ctxt, deviceBoot) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot count boot devices)); goto cleanup; @@ -10009,6 +10007,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i n ; i++) { virDomainRedirdevDefPtr redirdev = virDomainRedirdevDefParseXML(nodes[i], + bootMap, flags); if (!redirdev) goto error; @@ -13449,7 +13448,8 @@ virDomainRedirdevDefFormat(virBufferPtr buf, virBufferAsprintf(buf, redirdev bus='%s', bus); if (virDomainChrSourceDefFormat(buf, def-source.chr, false, flags) 0) return -1; -if (virDomainDeviceInfoFormat(buf, def-info, flags) 0) +if (virDomainDeviceInfoFormat(buf, def-info, + flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) 0) return -1; virBufferAddLit(buf, /redirdev\n); -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] tests: update qemuhelptest data
Both generated with qemu-system-x86_64 --help qemu-1.2.0 qemu-system-x86_64 \ -device ? \ -device pci-assign,? \ -device virtio-blk-pci,? \ -device virtio-net-pci,? \ -device scsi-disk,? \ -device PIIX4_PM,? \ -device usb-redir,? \ -device ide-drive,? \ -device usb-host,? 2 qemu-1.2.0-device It seems I missed a few -device flags when doing this last time and I mixed up qemu and qemu-kvm. --- tests/qemuhelpdata/qemu-1.2.0|1 + tests/qemuhelpdata/qemu-1.2.0-device | 51 +-- tests/qemuhelpdata/qemu-kvm-1.2.0| 276 ++ tests/qemuhelpdata/qemu-kvm-1.2.0-device | 222 tests/qemuhelptest.c | 106 +++- 5 files changed, 644 insertions(+), 12 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-kvm-1.2.0 create mode 100644 tests/qemuhelpdata/qemu-kvm-1.2.0-device diff --git a/tests/qemuhelpdata/qemu-1.2.0 b/tests/qemuhelpdata/qemu-1.2.0 index f52fdcb..78375f3 100644 --- a/tests/qemuhelpdata/qemu-1.2.0 +++ b/tests/qemuhelpdata/qemu-1.2.0 @@ -175,6 +175,7 @@ Character device options: -chardev stdio,id=id[,mux=on|off][,signal=on|off] -chardev tty,id=id,path=path[,mux=on|off] -chardev parport,id=id,path=path[,mux=on|off] +-chardev spicevmc,id=id,name=name[,debug=debug] -iscsi [user=user][,password=password] [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE diff --git a/tests/qemuhelpdata/qemu-1.2.0-device b/tests/qemuhelpdata/qemu-1.2.0-device index 9230a93..5613e00 100644 --- a/tests/qemuhelpdata/qemu-1.2.0-device +++ b/tests/qemuhelpdata/qemu-1.2.0-device @@ -8,6 +8,7 @@ name esp, bus System name sysbus-ohci, bus System, desc OHCI USB Controller name virtio-blk-pci, bus PCI, alias virtio-blk name usb-uas, bus usb-bus +name qxl-vga, bus PCI, desc Spice QXL GPU (primary, vga compatible) name ide-drive, bus IDE, desc virtual IDE disk or CD-ROM (legacy) name x3130-upstream, bus PCI, desc TI X3130 Upstream Port of PCI Express Switch name cirrus-vga, bus PCI, desc Cirrus CLGD 54xx VGA @@ -32,6 +33,7 @@ name pci-ohci, bus PCI, desc Apple USB Controller name nec-usb-xhci, bus PCI name xio3130-downstream, bus PCI, desc TI X3130 Downstream Port of PCI Express Switch name virtserialport, bus virtio-serial-bus +name usb-redir, bus usb-bus name usb-braille, bus usb-bus name scsi-cd, bus SCSI, desc virtual SCSI CD-ROM name usb-wacom-tablet, bus usb-bus, desc QEMU PenPartner Tablet @@ -64,9 +66,9 @@ name usb-bt-dongle, bus usb-bus name usb-tablet, bus usb-bus name isa-vga, bus ISA name usb-kbd, bus usb-bus -name kvm-pci-assign, bus PCI, alias pci-assign, desc KVM-based PCI passthrough name isa-applesmc, bus ISA name rtl8139, bus PCI +name qxl, bus PCI, desc Spice QXL GPU (secondary) name i82557a, bus PCI, desc Intel i82557A Ethernet name i82557c, bus PCI, desc Intel i82557C Ethernet name usb-audio, bus usb-bus @@ -95,16 +97,6 @@ name vt82c686b-usb-uhci, bus PCI name usb-ehci, bus PCI name i6300esb, bus PCI name virtio-scsi-pci, bus PCI -kvm-pci-assign.host=pci-host-devaddr -kvm-pci-assign.prefer_msi=on/off -kvm-pci-assign.share_intx=on/off -kvm-pci-assign.bootindex=int32 -kvm-pci-assign.configfd=string -kvm-pci-assign.addr=pci-devfn -kvm-pci-assign.romfile=string -kvm-pci-assign.rombar=uint32 -kvm-pci-assign.multifunction=on/off -kvm-pci-assign.command_serr_enable=on/off virtio-blk-pci.class=hex32 virtio-blk-pci.drive=drive virtio-blk-pci.logical_block_size=blocksize @@ -179,3 +171,40 @@ scsi-disk.wwn=hex64 scsi-disk.channel=uint32 scsi-disk.scsi-id=uint32 scsi-disk.lun=uint32 +PIIX4_PM.smb_io_base=uint32 +PIIX4_PM.disable_s3=uint8 +PIIX4_PM.disable_s4=uint8 +PIIX4_PM.s4_val=uint8 +PIIX4_PM.addr=pci-devfn +PIIX4_PM.romfile=string +PIIX4_PM.rombar=uint32 +PIIX4_PM.multifunction=on/off +PIIX4_PM.command_serr_enable=on/off +usb-redir.chardev=chr +usb-redir.debug=uint8 +usb-redir.filter=string +usb-redir.bootindex=int32 +usb-redir.port=string +usb-redir.full-path=on/off +ide-drive.drive=drive +ide-drive.logical_block_size=blocksize +ide-drive.physical_block_size=blocksize +ide-drive.min_io_size=uint16 +ide-drive.opt_io_size=uint32 +ide-drive.bootindex=int32 +ide-drive.discard_granularity=uint32 +ide-drive.ver=string +ide-drive.wwn=hex64 +ide-drive.serial=string +ide-drive.model=string +ide-drive.unit=uint32 +usb-host.hostbus=uint32 +usb-host.hostaddr=uint32 +usb-host.hostport=string +usb-host.vendorid=hex32 +usb-host.productid=hex32 +usb-host.isobufs=uint32 +usb-host.bootindex=int32 +usb-host.pipeline=on/off +usb-host.port=string +usb-host.full-path=on/off diff --git a/tests/qemuhelpdata/qemu-kvm-1.2.0 b/tests/qemuhelpdata/qemu-kvm-1.2.0 new file mode 100644 index 000..372002a --- /dev/null +++ b/tests/qemuhelpdata/qemu-kvm-1.2.0 @@ -0,0 +1,276 @@ +QEMU emulator version 1.2.0 (qemu-kvm-devel), Copyright (c) 2003-2008 Fabrice Bellard +usage: qemu-system-x86_64 [options] [disk_image] + +'disk_image' is a raw hard disk image for IDE hard disk 0 + +Standard
[libvirt] [PATCH 2/4] docs: boot order for host and redirected USB devices
And a few spaces. --- docs/formatdomain.html.in | 27 +++ 1 files changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c8da33d..6a3b976 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2192,7 +2192,8 @@ boot sequence. The per-device codeboot/code elements cannot be used together with general boot elements in a href=#elementsOSBIOSBIOS bootloader/a section. - span class=sinceSince 0.8.8/span/dd + span class=sinceSince 0.8.8/span for PCI devices, + span class=sinceSince 1.0.1/span for USB devices. dtcoderom/code/dt ddThe coderom/code element is used to change how a PCI device's ROM is presented to the guest. The optional codebar/code @@ -2237,6 +2238,7 @@ lt;devicesgt; lt;redirdev bus='usb' type='tcp'gt; lt;source mode='connect' host='localhost' service='4000'/gt; + lt;boot order='1'/gt; lt;/redirdevgt; lt;redirfiltergt; lt;usbdev class='0x08' vendor='0x1234' product='0xbeef' version='2.00' allow='yes'/gt; @@ -2258,24 +2260,33 @@ tunnel; codetype='tcp'/code or codetype='spicevmc'/code (which uses the usbredir channel of a a href=#elementsGraphicsSPICE graphics -device/a) are typical.The redirdev element has an optional -sub-elementcodelt;addressgt;/code which can tie the +device/a) are typical. The redirdev element has an optional +sub-element codelt;addressgt;/code which can tie the device to a particular controller. Further sub-elements, such as codelt;sourcegt;/code, may be required according to the given type, although a codelt;targetgt;/code sub-element is not required (since the consumer of the character device is -the hypervisor itself,rather than a device visible in the guest). +the hypervisor itself, rather than a device visible in the guest). + /dd + dtcodeboot/code/dt + + ddSpecifies that the device is bootable. +The codeorder/code attribute determines the order in which +devices will be tried during boot sequence. The per-device +codeboot/code elements cannot be used together with general +boot elements in a href=#elementsOSBIOSBIOS bootloader/a section. +(span class=sinceSince 1.0.1/span) /dd dtcoderedirfilter/code/dt ddThecode redirfilter /codeelement is used for creating the filter rule to filter out certain devices from redirection. -It uses sub-element codelt;usbdevgt;/codeto define each filter rule. -codeclass/codeattribute is the USB Class code, for example, +It uses sub-element codelt;usbdevgt;/code to define each filter rule. +codeclass/code attribute is the USB Class code, for example, 0x08 represents mass storage devices. The USB device can be addressed by -vendor / product id using thecodevendor/code and codeproduct/code attributes. +vendor / product id using the codevendor/code and codeproduct/code attributes. codeversion/code is the bcdDevice value of USB device, such as 1.00, 1.10 and 2.00. These four attributes are optional and code-1/code can be used to allow -any value for them. codeallow/codeattribute is mandatory, +any value for them. codeallow/code attribute is mandatory, 'yes' means allow, 'no' for deny. /dd /dl -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Introduce virDomainFSTrim() public API
This will call FITRIM within guest. The API has 4 arguments, however, only 2 will be used for now (@dom and @minumum). The rest two are there if in future qemu guest agent learns them. --- include/libvirt/libvirt.h.in |4 +++ src/driver.h |6 + src/libvirt.c| 50 ++ src/libvirt_public.syms |5 4 files changed, 65 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 49a361a..b161bce 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4438,6 +4438,10 @@ int virDomainOpenGraphics(virDomainPtr dom, int virDomainInjectNMI(virDomainPtr domain, unsigned int flags); +int virDomainFSTrim(virDomainPtr dom, +const char *mountPoint, +unsigned long long minimum, +unsigned int flags); /** * virSchedParameterType: diff --git a/src/driver.h b/src/driver.h index 7ba66ad..5163840 100644 --- a/src/driver.h +++ b/src/driver.h @@ -903,6 +903,11 @@ typedef int unsigned char **cpumap, unsigned int *online, unsigned int flags); +typedef int +(*virDrvDomainFSTrim)(virDomainPtr dom, + const char *mountPoint, + unsigned long long minimum, + unsigned int flags); /** * _virDriver: @@ -1094,6 +1099,7 @@ struct _virDriver { virDrvNodeGetMemoryParameters nodeGetMemoryParameters; virDrvNodeSetMemoryParameters nodeSetMemoryParameters; virDrvNodeGetCPUMap nodeGetCPUMap; +virDrvDomainFSTrim domainFSTrim; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index bdb1dc6..1c6863f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20164,3 +20164,53 @@ error: virDispatchError(conn); return -1; } + +/** + * virDomainFSTrim: + * @dom: a domain object + * @mountPoint: which mount point trim + * @minimum: Minimum contiguous free range to discard in bytes + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Calls FITRIM within the guest (hence guest agent may be + * required depending on hypervisor used). Either call it on each + * mounted filesystem (@mountPoint is NULL) or just on specified + * @mountPoint. @minimum tell that free ranges smaller than this + * may be ignored (this is a hint and the guest may not respect + * it). By increasing this value, the fstrim operation will + * complete more quickly for filesystems with badly fragmented + * free space, although not all blocks will be discarded. + * + * Returns 0 on success, -1 otherwise. + */ +int +virDomainFSTrim(virDomainPtr dom, +const char *mountPoint, +unsigned long long minimum, +unsigned int flags) +{ +VIR_DOMAIN_DEBUG(dom, mountPoint=%s, minimum=%llu, flags=%x, + mountPoint, minimum, flags); + +virResetLastError(); + +if (!VIR_IS_DOMAIN(dom)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} + +if (dom-conn-driver-domainFSTrim) { +int ret = dom-conn-driver-domainFSTrim(dom, mountPoint, + minimum, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(dom-conn); +return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f494821..f699e93 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -574,4 +574,9 @@ LIBVIRT_1.0.0 { virNodeGetCPUMap; } LIBVIRT_0.10.2; +LIBVIRT_1.0.1 { +global: +virDomainFSTrim; +} LIBVIRT_1.0.0; + # define new API here using predicted next version number -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] tests: add boot order for host and redirected USB to qemu argv test
--- ...muxml2argv-hostdev-usb-address-device-boot.args |6 +++ ...emuxml2argv-hostdev-usb-address-device-boot.xml | 28 + .../qemuxml2argv-usb-redir-boot.args | 10 + .../qemuxml2argv-usb-redir-boot.xml| 42 tests/qemuxml2argvtest.c |9 5 files changed, 95 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args new file mode 100644 index 000..beb093c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.args @@ -0,0 +1,6 @@ +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 -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -usb -hda \ +/dev/HostVG/QEMUGuest1 \ +-device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.xml new file mode 100644 index 000..c11a963 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address-device-boot.xml @@ -0,0 +1,28 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ +/disk +hostdev mode='subsystem' type='usb' managed='no' + source +address bus='14' device='6'/ + /source + boot order='1'/ +/hostdev +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args new file mode 100644 index 000..3d0cc8f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.args @@ -0,0 +1,10 @@ +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 \ +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-chardev socket,id=charredir0,host=localhost,port=4000 \ +-device usb-redir,chardev=charredir0,id=redir0,bootindex=1 \ +-chardev spicevmc,id=charredir1,name=usbredir \ +-device usb-redir,chardev=charredir1,id=redir1,bootindex=2,bus=usb.0,port=4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.xml new file mode 100644 index 000..02a27c1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-boot.xml @@ -0,0 +1,42 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +controller type='usb' index='0' model='ich9-ehci1' + address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x7'/ +/controller +controller type='usb' index='0' model='ich9-uhci1' + master startport='0'/ + address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0' multifunction='on'/ +/controller +controller type='usb' index='0' model='ich9-uhci2' + master startport='2'/ + address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x1'/ +/controller +controller type='usb' index='0' model='ich9-uhci3' + master startport='4'/ + address type='pci' domain='0x' bus='0x00'
[libvirt] [PATCH 2/4] remote: Implement virDomainFSTrim
A new rule to fixup_name() in gendispatch.pl needs to be added, otherwise we are left with remoteDomainFstrim which is not wanted. --- src/remote/remote_driver.c |1 + src/remote/remote_protocol.x | 10 +- src/remote_protocol-structs |7 +++ src/rpc/gendispatch.pl |1 + 4 files changed, 18 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 62b7729..a8c6993 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6143,6 +6143,7 @@ static virDriver remote_driver = { .nodeSetMemoryParameters = remoteNodeSetMemoryParameters, /* 0.10.2 */ .nodeGetMemoryParameters = remoteNodeGetMemoryParameters, /* 0.10.2 */ .nodeGetCPUMap = remoteNodeGetCPUMap, /* 1.0.0 */ +.domainFSTrim = remoteDomainFSTrim, /* 1.0.1 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d6ac3c1..31567e2 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2682,6 +2682,13 @@ struct remote_node_get_cpu_map_ret { int ret; }; +struct remote_domain_fstrim_args { +remote_nonnull_domain dom; +remote_string mountPoint; +unsigned hyper minimum; +unsigned int flags; +}; + /*- Protocol. -*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -3026,7 +3033,8 @@ enum remote_procedure { REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */ -REMOTE_PROC_NODE_GET_CPU_MAP = 293 /* skipgen skipgen */ +REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ +REMOTE_PROC_DOMAIN_FSTRIM = 294 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 6fe7213..d0d4f53 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2139,6 +2139,12 @@ struct remote_node_get_cpu_map_ret { u_int online; intret; }; +struct remote_domain_fstrim_args { +remote_nonnull_domain dom; +remote_string mountPoint; +uint64_t minimum; +u_int flags; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2433,4 +2439,5 @@ enum remote_procedure { REMOTE_PROC_NETWORK_UPDATE = 291, REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, REMOTE_PROC_NODE_GET_CPU_MAP = 293, +REMOTE_PROC_DOMAIN_FSTRIM = 294, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index ae7ecba..899f4bc 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -44,6 +44,7 @@ sub fixup_name { $name =~ s/Os$/OS/; $name =~ s/Nmi$/NMI/; $name =~ s/Pm/PM/; +$name =~ s/Fstrim$/FSTrim/; return $name; } -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] qemu: Implement virDomainFSTrim
using qemu guest agent. As said in previous patch, @mountPoint must be NULL and @flags zero because qemu guest agent doesn't support these arguments yet. If qemu learns them, we can start supporting them as well. --- src/qemu/qemu_agent.c | 25 ++ src/qemu/qemu_agent.h |2 + src/qemu/qemu_driver.c | 83 3 files changed, 110 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index ab6dc22..6f517c9 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1448,3 +1448,28 @@ qemuAgentArbitraryCommand(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +int +qemuAgentFSTrim(qemuAgentPtr mon, +unsigned long long minimum) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fstrim, + U:minimum, minimum, + NULL); +if (!cmd) +return ret; + +ret = qemuAgentCommand(mon, cmd, reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK); + +if (reply ret == 0) +ret = qemuAgentCheckError(cmd, reply); + +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index c62c438..dad068b 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -83,4 +83,6 @@ int qemuAgentArbitraryCommand(qemuAgentPtr mon, const char *cmd, char **result, int timeout); +int qemuAgentFSTrim(qemuAgentPtr mon, +unsigned long long minimum); #endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 595c452..3dc5bdd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14733,6 +14733,88 @@ cleanup: return result; } +static int +qemuDomainFSTrim(virDomainPtr dom, + const char *mountPoint, + unsigned long long minimum, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm; +int ret = -1; +qemuDomainObjPrivatePtr priv; + +virCheckFlags(0, -1); + +if (mountPoint) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(Specifying mount point + is not supported for now)); +return -1; +} + +qemuDriverLock(driver); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +qemuDriverUnlock(driver); + +if (!vm) { +char uuidstr[VIR_UUID_STRING_BUFLEN]; +virUUIDFormat(dom-uuid, uuidstr); +virReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +priv = vm-privateData; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +goto cleanup; +} + +if (priv-agentError) { +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s, + _(QEMU guest agent is not + available due to an error)); +goto cleanup; +} + +if (!priv-agent) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(QEMU guest agent is not configured)); +goto cleanup; +} + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(domain is not running)); +goto endjob; +} + +qemuDomainObjEnterAgent(driver, vm); +ret = qemuAgentFSTrim(priv-agent, minimum); +qemuDomainObjExitAgent(driver, vm); +if (ret 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Failed to execute agent command)); +goto endjob; +} + +endjob: +if (qemuDomainObjEndJob(driver, vm) == 0) { +vm = NULL; +} + +cleanup: +if (vm) +virDomainObjUnlock(vm); +return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -14906,6 +14988,7 @@ static virDriver qemuDriver = { .nodeGetMemoryParameters = nodeGetMemoryParameters, /* 0.10.2 */ .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */ +.domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */ }; -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] virsh: Expose new virDomainFSTrim API
under domfstrim command. Since API doesn't support all parameters (some must be NULL or zero), don't expose these in virsh neither. --- tools/virsh-domain.c | 40 tools/virsh.pod | 12 2 files changed, 52 insertions(+), 0 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cc47383..4ea08f4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8300,6 +8300,45 @@ cleanup: return ret; } +static const vshCmdInfo info_domfstrim[] = { +{help, N_(Invoke fstrim on domain's mounted filesystems.)}, +{desc, N_(Invoke fstrim on domain's mounted filesystems.)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_domfstrim[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{minimum, VSH_OT_INT, 0, N_(Just a hint to ignore contiguous + free ranges smaller than this (Bytes))}, +{NULL, 0, 0, NULL} +}; +static bool +cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +bool ret = false; +unsigned long long minimum = 0; +unsigned int flags = 0; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +goto cleanup; + +if (vshCommandOptULongLong(cmd, minimum, minimum) 0) { +vshError(ctl, _(Unable to parse integer parameter)); +goto cleanup; +} + +if (virDomainFSTrim(dom, NULL, minimum, flags) 0) { +vshError(ctl, _(Unable to invoke fstrim)); +goto cleanup; +} + +ret = true; + +cleanup: +return ret; +} + const vshCmdDef domManagementCmds[] = { {attach-device, cmdAttachDevice, opts_attach_device, info_attach_device, 0}, @@ -8332,6 +8371,7 @@ const vshCmdDef domManagementCmds[] = { {detach-interface, cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, {domdisplay, cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, +{domfstrim, cmdDomFSTrim, opts_domfstrim, info_domfstrim, 0}, {domhostname, cmdDomHostname, opts_domhostname, info_domhostname, 0}, {domid, cmdDomid, opts_domid, info_domid, 0}, {domif-setlink, cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 29be39e..c3fdcb7 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -878,6 +878,18 @@ Output a URI which can be used to connect to the graphical display of the domain via VNC, SPICE or RDP. If I--include-password is specified, the SPICE channel password will be included in the URI. +=item Bdomfstrim Idomain [I--minimum Bbytes] + +Issue a fstrim command on all mounted filesystems within a running +domain. It discards blocks which are not in use by the filesystem. +If I--minimum Bbytes is specified, it tells guest kernel length +of contiguous free range. Smaller than this may be ignored (this is +a hint and the guest may not respect it). By increasing this value, +the fstrim operation will complete more quickly for filesystems +with badly fragmented free space, although not all blocks will +be discarded. The default value is zero, meaning discard +every free block. + =item Bdomhostname Idomain Returns the hostname of a domain, if the hypervisor makes it available. -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Introduce support for FITRIM within guest OS
https://bugzilla.redhat.com/show_bug.cgi?id=831159 Michal Privoznik (4): Introduce virDomainFSTrim() public API remote: Implement virDomainFSTrim qemu: Implement virDomainFSTrim virsh: Expose new virDomainFSTrim API include/libvirt/libvirt.h.in |4 ++ src/driver.h |6 +++ src/libvirt.c| 50 + src/libvirt_public.syms |5 +++ src/qemu/qemu_agent.c| 25 + src/qemu/qemu_agent.h|2 + src/qemu/qemu_driver.c | 83 ++ src/remote/remote_driver.c |1 + src/remote/remote_protocol.x | 10 +- src/remote_protocol-structs |7 src/rpc/gendispatch.pl |1 + tools/virsh-domain.c | 40 tools/virsh.pod | 12 ++ 13 files changed, 245 insertions(+), 1 deletions(-) -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] gobject: Also delete storage volume from its pool's list
From: Zeeshan Ali (Khattak) zeesha...@gnome.org Without this patch, storage pool still lists the volume even after it is deleted. Related Boxes bug: https://bugzilla.gnome.org/show_bug.cgi?id=688724 --- libvirt-gobject/libvirt-gobject-storage-pool.c | 23 +++ libvirt-gobject/libvirt-gobject-storage-vol.c | 4 2 files changed, 27 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 8f579a1..df7e77c 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -29,6 +29,7 @@ #include libvirt-glib/libvirt-glib.h #include libvirt-gobject/libvirt-gobject.h #include libvirt-gobject-compat.h +#include libvirt-gobject-storage-pool-private.h #define GVIR_STORAGE_POOL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_POOL, GVirStoragePoolPrivate)) @@ -1129,3 +1130,25 @@ gboolean gvir_storage_pool_delete_finish(GVirStoragePool *pool, return TRUE; } + +G_GNUC_INTERNAL gboolean gvir_storage_pool_delete_vol(GVirStoragePool *pool, + GVirStorageVol *volume) +{ +GVirStoragePoolPrivate *priv; +gboolean ret = FALSE; + +g_return_val_if_fail(GVIR_IS_STORAGE_POOL(pool), FALSE); +g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume), FALSE); + +priv = pool-priv; +g_mutex_lock(priv-lock); +if (priv-volumes != NULL) { +const gchar *name = gvir_storage_vol_get_name(volume); +ret = g_hash_table_remove(priv-volumes, name); +} else { +g_warn_if_reached(); +} +g_mutex_unlock(priv-lock); + +return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index c7ebb45..4352d30 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -29,6 +29,7 @@ #include libvirt-glib/libvirt-glib.h #include libvirt-gobject/libvirt-gobject.h #include libvirt-gobject-compat.h +#include libvirt-gobject-storage-pool-private.h #define GVIR_STORAGE_VOL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_STORAGE_VOL, GVirStorageVolPrivate)) @@ -309,6 +310,9 @@ gboolean gvir_storage_vol_delete(GVirStorageVol *vol, g_return_val_if_fail(GVIR_IS_STORAGE_VOL(vol), FALSE); g_return_val_if_fail(err == NULL || *err == NULL, FALSE); +if (!gvir_storage_pool_delete_vol(vol-priv-pool, vol)) +return FALSE; + if (virStorageVolDelete(vol-priv-handle, flags) 0) { gvir_set_error_literal(err, GVIR_STORAGE_VOL_ERROR, -- 1.8.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] Introduce virDomainFSTrim() public API
On Tue, Nov 20, 2012 at 07:47:35PM +0100, Michal Privoznik wrote: This will call FITRIM within guest. The API has 4 arguments, however, only 2 will be used for now (@dom and @minumum). The rest two are there if in future qemu guest agent learns them. --- include/libvirt/libvirt.h.in |4 +++ src/driver.h |6 + src/libvirt.c| 50 ++ src/libvirt_public.syms |5 4 files changed, 65 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 49a361a..b161bce 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4438,6 +4438,10 @@ int virDomainOpenGraphics(virDomainPtr dom, int virDomainInjectNMI(virDomainPtr domain, unsigned int flags); +int virDomainFSTrim(virDomainPtr dom, +const char *mountPoint, +unsigned long long minimum, +unsigned int flags); /** * virSchedParameterType: diff --git a/src/driver.h b/src/driver.h index 7ba66ad..5163840 100644 --- a/src/driver.h +++ b/src/driver.h @@ -903,6 +903,11 @@ typedef int unsigned char **cpumap, unsigned int *online, unsigned int flags); +typedef int +(*virDrvDomainFSTrim)(virDomainPtr dom, + const char *mountPoint, + unsigned long long minimum, + unsigned int flags); /** * _virDriver: @@ -1094,6 +1099,7 @@ struct _virDriver { virDrvNodeGetMemoryParameters nodeGetMemoryParameters; virDrvNodeSetMemoryParameters nodeSetMemoryParameters; virDrvNodeGetCPUMap nodeGetCPUMap; +virDrvDomainFSTrim domainFSTrim; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index bdb1dc6..1c6863f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20164,3 +20164,53 @@ error: virDispatchError(conn); return -1; } + +/** + * virDomainFSTrim: + * @dom: a domain object + * @mountPoint: which mount point trim + * @minimum: Minimum contiguous free range to discard in bytes + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Calls FITRIM within the guest (hence guest agent may be + * required depending on hypervisor used). Either call it on each + * mounted filesystem (@mountPoint is NULL) or just on specified + * @mountPoint. @minimum tell that free ranges smaller than this + * may be ignored (this is a hint and the guest may not respect + * it). By increasing this value, the fstrim operation will + * complete more quickly for filesystems with badly fragmented + * free space, although not all blocks will be discarded. + * + * Returns 0 on success, -1 otherwise. + */ +int +virDomainFSTrim(virDomainPtr dom, +const char *mountPoint, +unsigned long long minimum, +unsigned int flags) +{ +VIR_DOMAIN_DEBUG(dom, mountPoint=%s, minimum=%llu, flags=%x, + mountPoint, minimum, flags); + +virResetLastError(); + +if (!VIR_IS_DOMAIN(dom)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} You're missing the check for read-only connections + +if (dom-conn-driver-domainFSTrim) { +int ret = dom-conn-driver-domainFSTrim(dom, mountPoint, + minimum, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(dom-conn); +return -1; +} 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] [PATCH] Log an audit message with the LXC init pid
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/20/2012 12:52 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently the LXC driver logs audit messages when a container is started or stopped. These audit messages, however, contain the PID of the libvirt_lxc supervisor process. To enable sysadmins to correlate with audit messages generated by processes /inside/ the container, we need to include the container init process PID. We can't do this in the main 'start' audit message, since the init PID is not available at that point. Instead we output a completely new audit record, that lists both PIDs. type=VIRT_CONTROL msg=audit(1353433750.071:363): pid=20180 uid=0 auid=501 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='virt=lxc op=init vm=busy uuid=dda7b947-0846-1759-2873-0f375df7d7eb vm-pid=20371 init-pid=20372 exe=/home/berrange/src/virt/libvirt/daemon/.libs/lt-libvirtd hostname=? addr=? terminal=pts/6 res=success' --- src/conf/domain_audit.c | 26 ++ src/conf/domain_audit.h | 3 +++ src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 31 ++- src/lxc/lxc_monitor.c| 23 +++ src/lxc/lxc_monitor.h| 5 + src/lxc/lxc_process.c| 8 src/lxc/lxc_protocol.x | 7 ++- 8 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 0f3924a..34cd92e 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -605,6 +605,32 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) virDomainAuditLifecycle(vm, start, reason, success); } +void +virDomainAuditInit(virDomainObjPtr vm, + unsigned long long initpid) +{ +char uuidstr[VIR_UUID_STRING_BUFLEN]; +char *vmname; +const char *virt; + +virUUIDFormat(vm-def-uuid, uuidstr); + +if (!(vmname = virAuditEncode(vm, vm-def-name))) { + VIR_WARN(OOM while encoding audit message); +return; +} + + if (!(virt = virDomainVirtTypeToString(vm-def-virtType))) { + VIR_WARN(Unexpected virt type %d while encoding audit message, vm-def-virtType); +virt = ?; +} + + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, true, + virt=%s op=init %s uuid=%s vm-pid=%lld init-pid=%lld, + virt, vmname, uuidstr, (long long)vm-pid, initpid); + +VIR_FREE(vmname); +} void virDomainAuditStop(virDomainObjPtr vm, const char *reason) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index db35d09..94b1198 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -31,6 +31,9 @@ void virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void virDomainAuditInit(virDomainObjPtr vm, +unsigned long long pid) +ATTRIBUTE_NONNULL(1); void virDomainAuditStop(virDomainObjPtr vm, const char *reason) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5a07139..23369e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -258,6 +258,7 @@ virDomainAuditCgroupPath; virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; +virDomainAuditInit; virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4746897..65d117a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -123,6 +123,7 @@ struct _virLXCController { /* Server socket */ virNetServerPtr server; +bool firstClient; virNetServerClientPtr client; virNetServerProgramPtr prog; bool inShutdown; @@ -132,6 +133,8 @@ struct _virLXCController { #include lxc_controller_dispatch.h static void virLXCControllerFree(virLXCControllerPtr ctrl); +static int virLXCControllerEventSendInit(virLXCControllerPtr ctrl, + pid_t initpid); static void virLXCControllerQuitTimer(int timer ATTRIBUTE_UNUSED, void *opaque) { @@ -152,6 +155,7 @@ static virLXCControllerPtr virLXCControllerNew(const char *name) goto no_memory; ctrl-timerShutdown = -1; +ctrl-firstClient = true; if (!(ctrl-name = strdup(name))) goto no_memory; @@ -588,6 +592,11 @@ static void *virLXCControllerClientPrivateNew(virNetServerClientPtr client, virNetServerClientSetCloseHook(client, virLXCControllerClientCloseHook); VIR_DEBUG(Got new client %p, client); ctrl-client = client; + +if (ctrl-initpid ctrl-firstClient) + virLXCControllerEventSendInit(ctrl, ctrl-initpid); +ctrl-firstClient = false; + return dummy; } @@ -1283,8 +1292,10 @@ virLXCControllerEventSend(virLXCControllerPtr ctrl, { virNetMessagePtr msg; -if (!ctrl-client) +if (!ctrl-client) { + VIR_WARN(Dropping event %d becuase libvirtd is not connected, procnr); return; +} VIR_DEBUG(Send event %d
Re: [libvirt] libvirt-1.0 fails virdrivermoduletest
On 11/19/2012 03:14 PM, Martin Kletzander wrote: On 11/11/2012 10:25 AM, Toralf Förster wrote: I've a stable Gentoo Linux, the build log is attached. It's hard to say from the log, but you can check yourselves what the problem is by running the particular test like this: VIR_TEST_DEBUG=1 ./virdrivermoduletest in the tests/ directory after compiling the source. That should give you an idea where the problem might be. Martin AH thx, I get : n22 /var/tmp/portage/app-emulation/libvirt-1.0.0/work/libvirt-1.0.0/tests # VIR_TEST_DEBUG=1 ./virdrivermoduletest TEST: virdrivermoduletest 1) Test driver storage ... OK 2) Test driver nodedev ... OK 3) Test driver secret ... OK 4) Test driver nwfilter... OK 5) Test driver interface ... OK 6) Test driver qemu... FAILED 7) Test driver lxc ... FAILED 8) Test driver uml ... OK and n22 /var/tmp/portage/app-emulation/libvirt-1.0.0/work/libvirt-1.0.0/tests # VIR_TEST_DEBUG=1 ./virnetsockettest TEST: virnetsockettest 1) Socket TCP/IPv4 Accept... libvir: XML-RPC error : End of file while reading data: Input/output error OK 2) Socket UNIX Accept... libvir: XML-RPC error : Cannot write data: Broken pipe OK 3) Socket UNIX Addrs ... OK 4) Socket External Command /dev/zero ... OK 5) Socket External Command /dev/does-not-exist ... libvir: XML-RPC error : End of file while reading data: /bin/cat: /dev/does-not-exist: No such file or directory: Input/output error OK 6) SSH test 1... libvir: XML-RPC error : End of file while reading data: ssh: Could not resolve hostname somehost: Name or service not known: Input/output error FAILED 7) SSH test 2... libvir: XML-RPC error : End of file while reading data: ssh: Could not resolve hostname somehost: Name or service not known: Input/output error FAILED 8) SSH test 3... libvir: XML-RPC error : End of file while reading data: ssh: Could not resolve hostname somehost: Name or service not known: Input/output error FAILED 9) SSH test 4... libvir: XML-RPC error : End of file while reading data: ssh: Could not resolve hostname nosuchhost: Name or service not known: Input/output error OK 10) SSH test 5... libvir: XML-RPC error : End of file while reading data: ssh: Could not resolve hostname crashyhost: Name or service not known: Input/output error FAILED 11) SSH test 6... libvir: XML-RPC error : End of file while reading data: Warning: Identity file /root/.ssh/example_key not accessible: No such file or directory. socket: Address family not supported by protocol ssh: connect to host example.com port 22: Address family not supported by protocol: Input/output error FAILED 12) SSH test 7... libvir: XML-RPC error : End of file while reading data: ssh: Could not resolve hostname somehost: Name or service not known: Input/output error FAILED hmm -- MfG/Sincerely Toralf Förster pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option
Laine mentioned something yesterday that got me to thinking: being able to specify that dnsmasq is not to be started for an interface. Let me expand that by saying that libvirt would not start dnsmasq for either dns or dhcp and also would not start radvd. However, the IPv4 and IPv6 gateway addresses would be defined on the virtual network interface and the usual iptables and ip6tables rules would be in force. This would allow a user to configure dnsmasq to meet any user desires or use something completely different instead of dnsmasq. Questions: Useful? Worth the time and effort? And then there is how should this be specified in the network xml file? ... some new parameter? ... A subperameter of dns such as dns disable='yes' / ? ... a subparameter of bridge such as bridge name=virbr0 dns=disable / ? Comments? Suggestions? Gene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option
Quoting Gene Czarcinski (g...@czarc.net): Laine mentioned something yesterday that got me to thinking: being able to specify that dnsmasq is not to be started for an interface. Let me expand that by saying that libvirt would not start dnsmasq for either dns or dhcp and also would not start radvd. However, the IPv4 and IPv6 gateway addresses would be defined on the virtual network interface and the usual iptables and ip6tables rules would be in force. This would allow a user to configure dnsmasq to meet any user desires or use something completely different instead of dnsmasq. Questions: Useful? Worth the time and effort? And then there is Useful, yes, it's been requested by Scott Moser before (https://www.redhat.com/archives/libvirt-users/2012-September/msg00095.html) how should this be specified in the network xml file? ... some new parameter? ... A subperameter of dns such as dns disable='yes' / ? ... a subparameter of bridge such as bridge name=virbr0 dns=disable / ? Comments? Suggestions? No opinions from me, both seem doable. Don't know if anyone will eventually want further dns(masq) customization. -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCHv2 4/n] snapshot: actually compute branch definition from XML
On 11/19/2012 05:09 PM, Eric Blake wrote: When asked to parse a branch snapshot XML definition, we have to piece together the definition of the new snapshot from parts of the branch point, as well as run some sanity checks that the branches are compatible. This patch is rather restrictive in what it allows; depending on effort and future needs, we may be able to relax some of those restrictions and allow more cases. +if (other) { +/* For now, we only allow branch snapshots of external snapshots. */ +if (virDomainSnapshotAlignDisks(def, + VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL, +true) 0) +goto cleanup; +for (i = 0; i def-ndisks; def++) You can tell I was sending this late yesterday, with blatant bugs like iterating the wrong variable. Now that I've actually tested the patch, and had valgrind point out the obvious, my v3 should be something that actually works. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option
On 11/20/2012 02:36 PM, Gene Czarcinski wrote: Laine mentioned something yesterday that got me to thinking: being able to specify that dnsmasq is not to be started for an interface. Let me expand that by saying that libvirt would not start dnsmasq for either dns or dhcp and also would not start radvd. However, the IPv4 and IPv6 gateway addresses would be defined on the virtual network interface and the usual iptables and ip6tables rules would be in force. This would allow a user to configure dnsmasq to meet any user desires or use something completely different instead of dnsmasq. Questions: Useful? Worth the time and effort? That was already determined before I mentioned it to you - it's been requested several times, and I've told some people it was going to happen, although didn't say when :-). And then there is how should this be specified in the network xml file? ... some new parameter? ... A subperameter of dns such as dns disable='yes' / ? ... a subparameter of bridge such as bridge name=virbr0 dns=disable / ? The bridge element is commonly *not* specified manually, but is filled in automatically be libvirt, so I don't think it's a good place to put optional flags (you would end up manually specifying settings for the things that are automatically filled in, like the bridge name). If anything, I would say the choice would be between putting it in dns or in the toplevel network, i.e.: network ... dns disable='yes'/ (or maybe dns enable='no'/ is better) ... /network or network dns='no' ... /network Or, maybe it would be even better to put it in the ip element: ip address='192.168.122.1' netmask='255.255.255.0' dns='no'/ that way you could have dnsmasq listen on some of the IP addresses defined for a network, but not others. At the same time, we probably to be able to disable ipv6 RA as well. Since that's only enabled when there is an ipv6 address, it can also be added into the ip element config: ip family='ipv6' address='f00d::1' ra='no'/ (or maybe autoconf='no' ?) Yeah, I think I like it best in ip (unless someone else has a better idea). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Porting Qemu driver to FreeBSD
On 11/20/2012 10:07 AM, Roman Bogorodskiy wrote: Hi, Recently I have picked up Sean's and Hiren's activities in porting Qemu driver on FreeBSD. At this point I have the basics working. Meaning that these things work: - Basic host commands (version, nodeinfo, etc) - Can perform basic operations on the VM (start, stop, destroy, etc) - Basic bridging (bridge creation/removing, adding/removing interfaces, setting addresses etc) Currently I keep FreeBSD specific info here: https://github.com/novel/fbsd-libvirt/blob/freebsd/README-freebsd Thanks for your porting efforts. Is this something where you have split it into a series of patches worth incorporating upstream? The code is available here: https://github.com/novel/fbsd-libvirt/tree/freebsd Current changes could be observed here: https://github.com/novel/fbsd-libvirt/compare/upstream_master...freebsd I would appreciate your thoughts and feedback on that topic. You're more likely to get feedback by posting actual patches here, rather than pointing us to a repo and expecting us to crawl through the differences. But here's hoping we can improve the code base as a result of your efforts. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 0/5] snapshot revert-and-create
v2 was here: https://www.redhat.com/archives/libvir-list/2012-November/msg00818.html One patch from v2 has already been committed. This patch additionally adds a qemu implementation for the new flag, and I have tested creation of offline branches (I still need to test creation of a branch from a disk-only or online external snapshot). I've also started documenting my plans for a new revert FOLLOW flag, which updates to the current state of external files (rather than reverting completely to the point where the snapshot was taken); once that is working, then implementing the combination of createXML(LIVE|BRANCH) will really be creating the branch, then farming out to revert(FOLLOW) to switch over to the new branch. Eric Blake (5): snapshot: add revert-and-create branching of external snapshots snapshot: prepare to parse new XML snapshot: actually compute branch definition from XML snapshot: support revert-and-create branching in qemu snapshot: add another revert API flag docs/formatsnapshot.html.in | 16 - docs/schemas/domainsnapshot.rng | 45 ++-- include/libvirt/libvirt.h.in | 6 ++ src/conf/snapshot_conf.c | 118 --- src/conf/snapshot_conf.h | 2 + src/esx/esx_driver.c | 2 +- src/libvirt.c| 41 +-- src/qemu/qemu_driver.c | 50 +++-- src/vbox/vbox_tmpl.c | 2 +- tests/domainsnapshotxml2xmlin/branch.xml | 5 ++ tests/domainsnapshotxml2xmltest.c| 2 +- tools/virsh-snapshot.c | 44 +--- tools/virsh.pod | 25 ++- 13 files changed, 300 insertions(+), 58 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlin/branch.xml -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/5] snapshot: prepare to parse new XML
No semantic change, but prepare for a new mode of parsing where a new _BRANCH flag requests that the parse look up the existing snapshot to branch from. * src/conf/snapshot_conf.h (VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH): New flag, unused for now. (virDomainSnapshotDefParseString): Add parameter. * src/conf/snapshot_conf.c (virDomainSnapshotDefParseString): Update signature. * src/esx/esx_driver.c (esxDomainSnapshotCreateXML): Update caller. * src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotCreateXML): Likewise. * tests/domainsnapshotxml2xmltest.c (testCompareXMLToXMLFiles): Likewise. --- src/conf/snapshot_conf.c | 8 +--- src/conf/snapshot_conf.h | 2 ++ src/esx/esx_driver.c | 2 +- src/qemu/qemu_driver.c| 3 ++- src/vbox/vbox_tmpl.c | 2 +- tests/domainsnapshotxml2xmltest.c | 2 +- 6 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 72bdd30..8a3146f 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -162,14 +162,16 @@ cleanup: return ret; } -/* flags is bitwise-or of virDomainSnapshotParseFlags. - * If flags does not include VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE, then - * caps and expectedVirtTypes are ignored. +/* flags is bitwise-or of virDomainSnapshotParseFlags. If flags does + * not include VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE, then 'caps' and + * 'expectedVirtTypes' are ignored. If flags does not include + * VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH, then 'snapshots' is ignored. */ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virCapsPtr caps, unsigned int expectedVirtTypes, +virDomainSnapshotObjListPtr snapshots ATTRIBUTE_UNUSED, unsigned int flags) { xmlXPathContextPtr ctxt = NULL; diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index b0f8760..c06f3b3 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -97,11 +97,13 @@ typedef enum { VIR_DOMAIN_SNAPSHOT_PARSE_DISKS= 1 1, VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL = 1 2, VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE = 1 3, +VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH = 1 4, } virDomainSnapshotParseFlags; virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virCapsPtr caps, unsigned int expectedVirtTypes, + virDomainSnapshotObjListPtr snapshots, unsigned int flags); void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def); char *virDomainSnapshotDefFormat(const char *domain_uuid, diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 56f31bb..3c869d7 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4267,7 +4267,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, return NULL; } -def = virDomainSnapshotDefParseString(xmlDesc, NULL, 0, 0); +def = virDomainSnapshotDefParseString(xmlDesc, NULL, 0, NULL, 0); if (def == NULL) { return NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d4cafcc..f5bbc52 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -509,7 +509,7 @@ qemuDomainSnapshotLoad(void *payload, def = virDomainSnapshotDefParseString(xmlStr, qemu_driver-caps, QEMU_EXPECTED_VIRT_TYPES, - flags); + NULL, flags); if (def == NULL) { /* Nothing we can do here, skip this one */ VIR_ERROR(_(Failed to parse snapshot XML from file '%s'), @@ -11511,6 +11511,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (!(def = virDomainSnapshotDefParseString(xmlDesc, driver-caps, QEMU_EXPECTED_VIRT_TYPES, +vm-snapshots, parse_flags))) goto cleanup; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index bcffb2f..4fd0505 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -5929,7 +5929,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, /* VBox has no snapshot metadata, so this flag is trivial. */ virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL); -if (!(def = virDomainSnapshotDefParseString(xmlDesc, NULL, 0, 0))) +if (!(def = virDomainSnapshotDefParseString(xmlDesc, NULL, 0, NULL, 0))) goto cleanup; if (def-ndisks) { diff --git a/tests/domainsnapshotxml2xmltest.c
[libvirt] [PATCHv3 3/5] snapshot: actually compute branch definition from XML
When asked to parse a branch snapshot XML definition, we have to piece together the definition of the new snapshot from parts of the branch point, as well as run some sanity checks that the branches are compatible. This patch is rather restrictive in what it allows; depending on effort and future needs, we may be able to relax some of those restrictions and allow more cases. * src/conf/snapshot_conf.c (virDomainSnapshotDefParseString): Honor new flag. --- src/conf/snapshot_conf.c | 112 --- 1 file changed, 106 insertions(+), 6 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 8a3146f..909a750 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -171,7 +171,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virCapsPtr caps, unsigned int expectedVirtTypes, -virDomainSnapshotObjListPtr snapshots ATTRIBUTE_UNUSED, +virDomainSnapshotObjListPtr snapshots, unsigned int flags) { xmlXPathContextPtr ctxt = NULL; @@ -181,6 +181,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, xmlNodePtr *nodes = NULL; int i; char *creation = NULL, *state = NULL; +char *branch = NULL; struct timeval tv; int active; char *tmp; @@ -188,6 +189,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, char *memorySnapshot = NULL; char *memoryFile = NULL; bool offline = !!(flags VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE); +virDomainSnapshotObjPtr other = NULL; xml = virXMLParseCtxt(NULL, xmlStr, _((domain_snapshot)), ctxt); if (!xml) { @@ -223,7 +225,84 @@ virDomainSnapshotDefParseString(const char *xmlStr, def-description = virXPathString(string(./description), ctxt); -if (flags VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { +branch = virXPathString(string(./branch), ctxt); +memorySnapshot = virXPathString(string(./memory/@snapshot), ctxt); +memoryFile = virXPathString(string(./memory/@file), ctxt); + +if (flags VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH) { +if ((flags (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL)) || +!snapshots) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(unexpected parse request)); +goto cleanup; +} + +/* In order to form a branch, we must find the existing + * snapshot, and it must be external. */ +if (!branch) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(use of branch flag requires branch element)); +goto cleanup; +} +other = virDomainSnapshotFindByName(snapshots, branch); +if (!other) { +virReportError(VIR_ERR_XML_ERROR, _(could not find branch '%s'), + branch); +goto cleanup; +} + +if (!virDomainSnapshotIsExternal(other)) { +virReportError(VIR_ERR_XML_ERROR, + _(branch '%s' is not an external snapshot), + branch); +goto cleanup; +} +if (!other-def-dom) { +virReportError(VIR_ERR_XML_ERROR, + _(branch '%s' lacks corresponding domain details), + branch); +goto cleanup; +} +if (!(def-dom = virDomainDefCopy(caps, other-def-dom, true))) +goto cleanup; + +/* The new definition shares the same parent, state, and + * domain as what it is branching from. */ +def-creationTime = tv.tv_sec; +if (other-def-parent +!(def-parent = strdup(other-def-parent))) { +virReportOOMError(); +goto cleanup; +} +def-state = other-def-state; + +/* Branches reuse existing memory files (with bookkeeping to + * only delete the file when the last snapshot reference is + * deleted). Thus, it is an error to explicitly request + * memory with a branch. */ +if (memorySnapshot || memoryFile) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(cannot specify memory and branch together)); +goto cleanup; +} +def-memory = other-def-memory; +if (other-def-file +!(def-file = strdup(other-def-file))) { +virReportOOMError(); +goto cleanup; +} + +/* Any disk in the XML must be consistent with the branch + * point, and any disk not in the XML will be populated to + * match the branch; checked below. */ +} else if (flags VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { +if (branch) { +virReportError(VIR_ERR_XML_ERROR, +
[libvirt] [RFC PATCHv3 5/5] snapshot: add another revert API flag
Now that we can create external snapshot branches, we need to be able to switch between them. Add a new flag, which states that we will merely use the external files as-is (and assume that the user hasn't done any nasty hotplug or hotunplug of disks after that branch was created, since we don't have any way to track the guest ABI at a tip of a branch that we are about to leave). * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_REVERT_FOLLOW): New flag. * src/libvirt.c (virDomainRevertToSnapshot): Document it. * tools/virsh-snapshot.c (cmdDomainSnapshotRevert): Add --follow flag. * tools/virsh.pod (snapshot-revert): Document it. --- include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c| 14 +- tools/virsh-snapshot.c | 4 tools/virsh.pod | 9 - 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index be16629..0957300 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3897,6 +3897,9 @@ 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 */ +VIR_DOMAIN_SNAPSHOT_REVERT_FOLLOW = 1 3, /* Follow subsequent disk +state rather than point of +external snapshot */ } virDomainSnapshotRevertFlags; /* Revert the domain to a point-in-time snapshot. The diff --git a/src/libvirt.c b/src/libvirt.c index 3229133..d2447c3 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18727,12 +18727,24 @@ error: * into an inactive state, so transient domains require the use of one * of these two flags. * + * In the case of external snapshots, it is also possible to resume from + * the state of the external disks as modified after the snapshot was + * originally taken, by adding VIR_DOMAIN_SNAPSHOT_REVERT_FOLLOW to + * @flags. In particular, this is the only way to switch between two + * external branches as created with VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH + * by virDomainSnapshotCreateXML(). + * * 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. Since the very nature of keeping snapshots * implies the intent to roll back state, no additional confirmation is - * normally required for these lossy effects. + * normally required for these lossy effects. Users are advised to + * pause or shut down a domain and create a final snapshot at the tip of + * that branch before using revert, if it may be desirable to return to + * that branch later on (especially true when the current execution + * descends from an external snapshot, to ensure pending I/O has been + * flushed to disk for later use by VIR_DOMAIN_SNAPSHOT_REVERT_FOLLOW). * * However, there are two particular situations where reverting will * be refused by default, and where @flags must include diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 6232ec2..fed728c 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1696,6 +1696,8 @@ static const vshCmdOptDef opts_snapshot_revert[] = { {running, VSH_OT_BOOL, 0, N_(after reverting, change state to running)}, {paused, VSH_OT_BOOL, 0, N_(after reverting, change state to paused)}, {force, VSH_OT_BOOL, 0, N_(try harder on risky reverts)}, +{follow, VSH_OT_BOOL, 0, + N_(follow disk changes made since an external snapshot)}, {NULL, 0, 0, NULL} }; @@ -1714,6 +1716,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING; if (vshCommandOptBool(cmd, paused)) flags |= VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED; +if (vshCommandOptBool(cmd, follow)) +flags |= VIR_DOMAIN_SNAPSHOT_REVERT_FOLLOW; /* We want virsh snapshot-revert --force to work even when talking * to older servers that did the unsafe revert by default but * reject the flag, so we probe without the flag, and only use it diff --git a/tools/virsh.pod b/tools/virsh.pod index 38c691e..dc70e68 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2837,7 +2837,7 @@ Output the name of the parent snapshot, if any, for the given Isnapshot, or for the current snapshot with I--current. =item Bsnapshot-revert Idomain {Isnapshot | I--current} -[{I--running | I--paused}] [I--force] +[{I--running | I--paused}] [I--force] [I--follow] Revert the given domain to the snapshot specified by Isnapshot, or to the current snapshot with I--current. Be aware @@ -2870,6 +2870,13 @@ snapshot that uses a provably incompatible configuration, as well as with an inactive snapshot that is
Re: [libvirt] [PATCHv4 4/4] snapshot: qemu: Implement reverting of external snapshots
On 11/19/2012 04:51 PM, Eric Blake wrote: On 11/19/2012 09:06 AM, Peter Krempa wrote: This patch adds support for reverting of external snapshots. The support is somewhat limited yet (you can only revert to a snapshot that has no children or delete the children that would have their image chains invalidated). While reverting an external snapshot, the domain has to be taken offline as there's no possibility to start a migration from file on a running machine. This poses a few difficulties when the user has virt-viewer attached as appropriate events need to be re-emitted even if the machine doesn't change states. --- Adapt for the revert flag and a few minor fixes. +/* wipe and re-create disk images - qemu-img doesn't care if it exists*/ +if (qemuDomainSnapshotCreateInactiveExternal(driver, vm, snap, false) 0) +goto endjob; This comment says you are throwing away state, but without the use of any flag guarding things that the user meant to throw away state. I'm a bit worried that we're missing a flag here to state that we are reverting to the point of the external snapshot _and_ want to throw away all changes that happened in the external file. Also, does qemuDomainSnapshotCreateInactiveExternal(,,,false) actually do what you want here? I thought that invocation fails if the file already exists (and passing true assumes that the file already exists, but does not otherwise touch it). Do we instead need to change that boolean into an 'int', with multiple values (0 and 1 for when the user first creates the snapshot, depending on whether they passed the REUSE_EXT flag, and 2 for this function, which says to forcefully recreate)? After thinking a bit more, I think we need the following additional flags to virDomainRevertToSnapshot, and require that exactly one of these three mutually exclusive flags is present when reverting to an external snapshot: VIR_DOMAIN_SNAPSHOT_REVERT_FOLLOW - revert to the current state of disks after an external snapshot (I'm about to post work on this flag - yes, it will conflict with what you've done so far) VIR_DOMAIN_SNAPSHOT_REVERT_RESET - revert to the point where an external snapshot was taken, and reset the external files to be exactly that state again (the semantics of _this_ patch) VIR_DOMAIN_SNAPSHOT_REVERT_COMMIT - revert to the current state of disks after an external snapshot, but commit the state of the external files into the backing files. Not possible if there is another snapshot branched off the same backing file (unless we want to let _FORCE allow us to potentially invalidate those other branches) I can also see the use for a revert-and-delete operation rolled into one, via a new flag: VIR_DOMAIN_SNAPSHOT_REVERT_DISCARD - after reverting to a snapshot, delete it (that is, we no longer plan to revert to it again in the future). When mixed with FOLLOW, we keep the backing chain with no change to disk contents; when mixed with RESET, we keep the backing chain but reset the backing chain to start fresh; and when mixed with COMMIT, we shorten the backing chain back to its pre-snapshot length. [I was debating whether the combination delete-and-revert fits better as part of the revert operation, via the REVERT_DISCARD flag, or whether it fits better as part of the delete operation, via a new flag there; but if we make it part of delete, then delete has to learn whether the domain should be running, paused, or stopped after an associated revert, whereas revert already has that logic.] I'm still the most worried about the concept of whether we are blindly discarding user disk state since the snapshot was created, and whether we are properly recreating external files if that's what the user really wanted. I'm not sure whether to say ACK and then fix the fallout, or to wait a bit longer to see what else you come up with in the series. I guess we still have a few more development days before the 1.0.1 freeze to decide, so for now, I'd like to see what you have in store for snapshot deletion, and to also get my patches posted for snapshot revert-and-create, before I decide on this one. I think I could ACK this patch if you respun it to require my proposed RESET flag to match your intent of discarding user disk state. Also, before you decide too much, you'd better read up on my respin with my proposed FOLLOW flag. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 1/5] snapshot: add revert-and-create branching of external snapshots
Right now, libvirt refuses to revert to the state at the time of an external snapshot, because doing so would reset things so that the next time the domain boots, we are using the backing file; but modifying the backing file invalidates all qcow2 files that are based on top of it. There are three possibilities for lifting this restriction: 1. delete all snapshot metadata and qcow2 files that are invalidated by the revert (losing changes since the snapshot) 2. perform a block commit (such as with qemu-img commit) to merge the qcow2 file back into the backing file (keeping the changes since the snapshot, but losing the qcow2 files) 3. create NEW qcow2 files that wrap the same base file as the original snapshot (keeping the changes since the original snapshot) This patch documents the API for option 3, by adding a new flag to virDomainSnapshotCreateXML (the only snapshot-related function that takes XML, which is required to pass in new file names during the branch), and wires up virsh to expose it. Later patches will enhance virDomainRevertToSnapshot to cover options 1 and 2. API wise, an application wishing to do the revert-and-create operation must add a branchname/branch element to their domainsnapshot XML, and pass VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH in the flags argument. In virsh, snapshot-create gains a new boolean flag --branch that must match the XML passed in, and snapshot-create-as gains a new --branch=name argument along with a --current boolean for creating such XML. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH): New flag. * src/libvirt.c (virDomainSnapshotCreateXML): Document it, and enforce some mutual exclusion. * docs/formatsnapshot.html.in: Document the new branch element. * docs/schemas/domainsnapshot.rng: Allow new element. * tools/virsh-snapshot.c (cmdSnapshotCreate): Add --branch option. (cmdSnapshotCreateAs): Likewise, also add --current. * tools/virsh.pod (snapshot-create, snapshot-create-as): Document new usage. * tests/domainsnapshotxml2xmlin/branch.xml: New test file. --- docs/formatsnapshot.html.in | 16 +--- docs/schemas/domainsnapshot.rng | 45 ++-- include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c| 27 +++ tests/domainsnapshotxml2xmlin/branch.xml | 5 tools/virsh-snapshot.c | 40 ++-- tools/virsh.pod | 16 ++-- 7 files changed, 114 insertions(+), 38 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlin/branch.xml diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 8fcc04c..8350cb0 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -188,9 +188,19 @@ /dd dtcodeparent/code/dt ddThe parent of this snapshot. If present, this element -contains exactly one child element, name. This specifies the -name of the parent snapshot of this snapshot, and is used to -represent trees of snapshots. Readonly. +contains exactly one child element, codename/code. This +specifies the name of the parent snapshot of this snapshot, +and is used to represent trees of snapshots. Readonly. + /dd + dtcodebranch/code/dt + ddThe name of an existing external snapshot that forms the +branch point for this snapshot. Use of this element must be +paired with the codeVIR_DOMAIN_SNAPSHOT_CREATE_BRANCH/code +flag during creation, and it is not present on output. When +creating a branch snapshot, the same set of codedisks/code +must be external, and codememory/code must be omitted (it +will be copied from the branch +point). span class=sincesince 1.0.1/span. /dd dtcodedomain/code/dt ddThe domain that this snapshot was taken against. Older diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 45d55b5..37910d8 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -31,27 +31,32 @@ /element /optional optional - element name='memory' -choice - attribute name='snapshot' -choice - valueno/value - valueinternal/value -/choice - /attribute - group -optional - attribute name='snapshot' -valueexternal/value - /attribute -/optional -attribute name='file' - ref name='absFilePath'/ + choice +element name='branch' + text/ +/element +element name='memory' + choice +attribute name='snapshot' + choice +valueno/value +
[libvirt] [PATCHv3 4/5] snapshot: support revert-and-create branching in qemu
First cut at allowing snapshot branch creation. For now, the code requires that the snapshot be created but not activated (thus leaving the user on the original branch); this is because activating the new branch will require shared code with revert. Of course, until we allow reverting to a snapshot branch, this functionality feels more like a write-only interface (we can create the snapshot but not use it); but one thing at a time. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Support new flag. (qemuDomainSnapshotPrepare): Likewise. --- src/qemu/qemu_driver.c | 47 +-- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f5bbc52..a17ab62 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10901,7 +10901,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, qemuDomainObjPrivatePtr priv = vm-privateData; if (def-state == VIR_DOMAIN_DISK_SNAPSHOT -reuse !qemuCapsGet(priv-caps, QEMU_CAPS_TRANSACTION)) { +reuse !qemuCapsGet(priv-caps, QEMU_CAPS_TRANSACTION) +!(*flags VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(reuse is not supported with this QEMU binary)); goto cleanup; @@ -11015,7 +11016,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, if (external !active) *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; -if (def-state != VIR_DOMAIN_DISK_SNAPSHOT active) { +if (def-state != VIR_DOMAIN_DISK_SNAPSHOT active +!(*flags VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH)) { if (external == 1 || qemuCapsGet(priv-caps, QEMU_CAPS_TRANSACTION)) { *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC; @@ -11464,7 +11466,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT | VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE | VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC | - VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL); + VIR_DOMAIN_SNAPSHOT_CREATE_LIVE | + VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH, NULL); if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) !(flags VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)) { @@ -11473,12 +11476,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, return NULL; } -if (((flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) +if (((flags (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH)) !(flags VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) || (flags VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) update_current = false; if (flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; +else if (flags VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH) +parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_BRANCH; qemuDriverLock(driver); virUUIDFormat(domain-uuid, uuidstr); @@ -11519,7 +11525,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (flags VIR_DOMAIN_SNAPSHOT_CREATE_LIVE (!virDomainObjIsActive(vm) || def-memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL || - flags VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) { + flags (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH))) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, _(live snapshot creation is supported only with external checkpoints)); @@ -11636,6 +11643,22 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, align_match) 0) goto cleanup; } +} else if (flags VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH) { +/* XXX For now, we require that the new branch is not current + * (getting that to work will require sharing code with + * snapshot revert for (re-)starting the domain with correct + * events). */ +if (flags VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(branch snapshot creation does not yet support + the current flag)); +goto cleanup; +} +/* def-dom was already populated, and the disks aligned; but + * we still need to check that we can create the new disk + * wrappers. */ +if (qemuDomainSnapshotPrepare(vm, def, flags) 0) +goto cleanup; } else { /* Easiest way to clone inactive portion of vm-def is via * conversion in and back out of xml. */ @@ -11678,7 +11701,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (update_current) snap-def-current = true; if
[libvirt] [PATCH] storage: Improve virStorageBackendFileSystemStop
It's actually not used for DIR pool. So removing the checking. --- src/storage/storage_backend_fs.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 3322677..10daee3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -933,7 +933,7 @@ no_memory: * @conn connection to report errors against * @pool storage pool to start * - * Stops a directory or FS based storage pool. + * Stops a FS based storage pool. * * - If it is a FS based pool, unmounts the unlying source device on the pool * - Releases all cached data about volumes @@ -943,8 +943,7 @@ static int virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { -if (pool-def-type != VIR_STORAGE_POOL_DIR -virStorageBackendFileSystemUnmount(pool) 0) +if (virStorageBackendFileSystemUnmount(pool) 0) return -1; return 0; -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V8 1/6] add a configure option --with-fuse to prepare introduction of fuse support for libvirt lxc
Ping on 2012/11/12 15:02, Gao feng wrote: add a configure option --with-fuse to prepare introduction of fuse support for libvirt lxc. With help from Daniel and Richard. Signed-off-by: Gao feng gaof...@cn.fujitsu.com --- configure.ac| 29 + libvirt.spec.in |9 + 2 files changed, 38 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 9108ea8..495cbfa 100644 --- a/configure.ac +++ b/configure.ac @@ -115,6 +115,7 @@ LIBSSH2_REQUIRED=1.0 LIBSSH2_TRANSPORT_REQUIRED=1.3 LIBBLKID_REQUIRED=2.17 DBUS_REQUIRED=1.0.0 +FUSE_REQUIRED=2.8.6 dnl Checks for C compiler. AC_PROG_CC @@ -1859,6 +1860,29 @@ AC_SUBST([CAPNG_CFLAGS]) AC_SUBST([CAPNG_LIBS]) +dnl libfuse +AC_ARG_WITH([fuse], + AC_HELP_STRING([--with-fuse], [use libfuse to proivde fuse filesystem support for libvirt lxc]), + [], + [with_fuse=check]) +dnl +dnl This check looks for 'fuse' +dnl +AS_IF([test x$with_fuse != xno], +[PKG_CHECK_MODULES([FUSE], [fuse = $FUSE_REQUIRED], + [with_fuse=yes + AC_SUBST([FUSE_CFLAGS]) + AC_SUBST([FUSE_LIBS]) + AC_DEFINE_UNQUOTED([HAVE_FUSE], 1, [whether fuse is available for libvirt lxc]) + ], + [if test x$with_fuse = xyes ; then + AC_MSG_ERROR([You must install fuse library to compile libvirt]) + else + with_fuse=no + fi + ]) +]) +AM_CONDITIONAL([HAVE_FUSE], [test x$with_fuse = xyes]) dnl virsh libraries AC_CHECK_HEADERS([readline/readline.h]) @@ -3163,6 +3187,11 @@ AC_MSG_NOTICE([ capng: $CAPNG_CFLAGS $CAPNG_LIBS]) else AC_MSG_NOTICE([ capng: no]) fi +if test $with_fuse = yes ; then +AC_MSG_NOTICE([fuse: $FUSE_CFLAGS $FUSE_LIBS]) +else +AC_MSG_NOTICE([fuse: no]) +fi if test $with_xen = yes ; then AC_MSG_NOTICE([ xen: $XEN_CFLAGS $XEN_LIBS]) else diff --git a/libvirt.spec.in b/libvirt.spec.in index 9aa2fb2..2c2c77c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -93,6 +93,7 @@ # A few optional bits off by default, we enable later %define with_polkit0%{!?_without_polkit:0} %define with_capng 0%{!?_without_capng:0} +%define with_fuse 0%{!?_without_fuse:0} %define with_netcf 0%{!?_without_netcf:0} %define with_udev 0%{!?_without_udev:0} %define with_hal 0%{!?_without_hal:0} @@ -510,6 +511,9 @@ BuildRequires: numactl-devel %if %{with_capng} BuildRequires: libcap-ng-devel = 0.5.0 %endif +%if %{with_fuse} +BuildRequires: fuse-devel = 2.8.6 +%endif %if %{with_phyp} || %{with_libssh2_transport} %if %{with_libssh2_transport} BuildRequires: libssh2-devel = 1.3.0 @@ -1193,6 +1197,10 @@ of recent versions of Linux (and other OSes). %define _without_capng --without-capng %endif +%if ! %{with_fuse} +%define _without_fuse --without-fuse +%endif + %if ! %{with_netcf} %define _without_netcf --without-netcf %endif @@ -1296,6 +1304,7 @@ autoreconf -if %{?_without_numactl} \ %{?_without_numad} \ %{?_without_capng} \ + %{?_without_fuse} \ %{?_without_netcf} \ %{?_without_selinux} \ %{?_with_selinux_mount} \ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: Fix bug of fs pool destroying
Regression introduced by commit 258e06c85b7, ret could be set to 1 or 0 by virStorageBackendFileSystemIsMounted before goto cleanup. This could mislead the callers (up to the public API virStoragePoolDestroy) to return success even the underlying umount command fails. --- src/storage/storage_backend_fs.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 10daee3..cbcbe34 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -449,6 +449,7 @@ static int virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { virCommandPtr cmd = NULL; int ret = -1; +int rc; if (pool-def-type == VIR_STORAGE_POOL_NETFS) { if (pool-def-source.nhost != 1) { @@ -475,8 +476,8 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { } /* Short-circuit if already unmounted */ -if ((ret = virStorageBackendFileSystemIsMounted(pool)) != 1) { -if (ret 0) +if ((rc = virStorageBackendFileSystemIsMounted(pool)) != 1) { +if (rc 0) return -1; else return 0; -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Migration with NPIV
On 2012年11月20日 18:08, Daniel P. Berrange wrote: On Mon, Nov 19, 2012 at 06:42:42PM +0800, Osier Yang wrote: One API missed is: int virNodeDeviceCreate(virNodeDevicePtr dev, unsigned int flags); To create the vHBA. That API + functionality already exists The existed API virNodeDeviceCreateXML is to create the vHBA from provided XML. The proposed one is to start it with the node device object. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Migration with NPIV
On 2012年11月20日 18:17, Daniel P. Berrange wrote: On Mon, Nov 19, 2012 at 05:30:11PM +0800, Osier Yang wrote: Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce autostart for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); I don't really much like this approach. IMHO, this should all be done via the virStoragePool APIs instead. Adding define/undefine/autostart to virNodeDevice is really just duplicating the storage pool functionality. Agreed, though it means I have to quit the nearly finished patches. Actually I'm not comfortable with the way either while facing the conflicts between the device configurations probed by udev or HAL and the persistent configuration trying to support. So the left work is to improve storage pool's XML so that the vHBA it refers to could be stable. And also manage the lifecyle of vHBA with pool's lifecyle. For how to make sure the pool should not be destroyed if there is volume of the pool is being used by domain, IMO it's time to integrate the storage pool with domain? I.e mapping storage volume to domain disk, and ref/unref the storage volume with domain's lifecyle. 2) Associate vHBA with domain XML There are two ways to attach a LUN to a domain: as an QEMU emulated device; or passthrough. Since passthrough a LUN is not supported in libvirt yet, let's focus on the emulated LUN at this stage. New attributes wwnn and wwpn are introduced to indicate the LUN behind the vHBA. E.g. disk type='block' device='disk' driver name='qemu' type='raw'/ source wwnn=2001001b32a9da4e wwpn=2101001b32a90004/ If you change the schema of thesource element, then you must also create a new type='XXX' attribute to identify it, not just re-use type='block' target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /disk Before the domain starting, we have to check if there is LUN assigned to the vHBA, error out if not. Using the stable path of LUN also works, e.g. source dev=/dev/disk/by-path/pci-\:00\:07.0-scsi-0\:0\:0\:0/ But the disadvantage is the user have to figure out the stable path himself; And we have to do checking of every stable path to see if it's behind a vHBA in migration Begin stage. Or an new XML tag for element source to indicate that it's behind a vHBA? such as: source dev=disk-by-path model=vport/ I don't much like the idea of mapping vHBA todisk elements, because you have a cardinality mis-match. Adisk is equivalent of a single LUN, but a vHBA is something that provides multiple LUNs. If you want to directly associate a vHBA with a virtual guest, then this is really in the realm of SCSI HBA passthrough, not disk devices. Agreed, I missed that multiple LUNs can be mapped to one HBA. If you want something mapped to thedisk device, then the approach should be to map to a storage pool volume - something we've long talked about as broadly useful for all storage types, not just NPIV. Okay, finally we are at the point to integerate storage with domain. 3) Migration with vHBA One possible solution for migration with vHBA is to use one pair of WWNN WWPN on source host, one is using for domain, one is reserved for migration purpose. It requires the storage admin maps the same LUN to the two vHBAs when doing the masking and zoning. One of the two vHBA
Re: [libvirt] RFC: Migration with NPIV
On 2012年11月21日 00:26, Dave Allan wrote: On Tue, Nov 20, 2012 at 10:17:11AM +, Daniel P. Berrange wrote: On Mon, Nov 19, 2012 at 05:30:11PM +0800, Osier Yang wrote: Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce autostart for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); I don't really much like this approach. IMHO, this should all be done via the virStoragePool APIs instead. Adding define/undefine/autostart to virNodeDevice is really just duplicating the storage pool functionality. I like the idea of making vHBAs persist as part of pools; how do you envision it should work? Extend the scsi pools to take a vHBA descriptor and then instantiating the vHBA as part of starting the pool, or something else? 2) Associate vHBA with domain XML There are two ways to attach a LUN to a domain: as an QEMU emulated device; or passthrough. Since passthrough a LUN is not supported in libvirt yet, let's focus on the emulated LUN at this stage. New attributes wwnn and wwpn are introduced to indicate the LUN behind the vHBA. E.g. disk type='block' device='disk' driver name='qemu' type='raw'/ source wwnn=2001001b32a9da4e wwpn=2101001b32a90004/ If you change the schema of thesource element, then you must also create a new type='XXX' attribute to identify it, not just re-use type='block' target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /disk Before the domain starting, we have to check if there is LUN assigned to the vHBA, error out if not. Using the stable path of LUN also works, e.g. source dev=/dev/disk/by-path/pci-\:00\:07.0-scsi-0\:0\:0\:0/ But the disadvantage is the user have to figure out the stable path himself; And we have to do checking of every stable path to see if it's behind a vHBA in migration Begin stage. Or an new XML tag for element source to indicate that it's behind a vHBA? such as: source dev=disk-by-path model=vport/ I don't much like the idea of mapping vHBA todisk elements, because you have a cardinality mis-match. Adisk is equivalent of a single LUN, but a vHBA is something that provides multiple LUNs. If you want to directly associate a vHBA with a virtual guest, then this is really in the realm of SCSI HBA passthrough, not disk devices. If you want something mapped to thedisk device, then the approach should be to map to a storage pool volume - something we've long talked about as broadly useful for all storage types, not just NPIV. +1, we really should take this as an opportunity to add storage volumes asdisk devices. 3) Migration with vHBA One possible solution for migration with vHBA is to use one pair of WWNN WWPN on source host, one is using for domain, one is reserved for migration purpose. It requires the storage admin maps the same LUN to the two vHBAs when doing the masking and zoning. One of the two vHBA is called Primary vHBA, another is called secondary vHBA. To maitain the relationship between these two vHBAs, we have to introduce new XMLs to vHBA. E.g. In XML of primary vHBA: secondary wwpn=2101001b32a90004/ In XML of secondary vHBA: primary wwpn=2101001b32a90002/ Primary vHBA is going to be guaranteed not used by any domain which is driven by libvirt (we do some checking eariler before the domain starting). And
Re: [libvirt] RFC: Migration with NPIV
On 2012年11月20日 09:36, Zou, Yi wrote: On 2012年11月19日 17:30, Osier Yang wrote: Hi, This proposal is trying to figure out a solution for migration of domain which uses LUN behind vHBA as disk device (QEMU emulated disk only at this stage). And other related NPIV improvements which are not related with migration. I'm not luck to get a environment to test if the thoughts are workable, but I'd like see if guys have good idea/suggestions earlier. Glad to see this topic on the list. 1) Persistent vHBA support This is the useful stuff missed for long time. Assuming that one created a vHBA, did masking/zoning, everything works as expected. However, after a system rebooting, everything is just lost. If the user wants to get things back, he has to find out the preivous WWNN WWPN, and create the vHBA again. On the other hand, Persistent vHBA support is actually required for domain which uses LUN behind a vHBA. Othewise the domain could fail to start after a system rebooting. To support the persistent vHBA, new APIs like virNodeDeviceDefineXML, virNodeDeviceUndefine is required. Also it's useful to introduce autostart for vHBA, so that the vHBA could be started automatically after system rebooting. Proposed APIs: virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virNodeDeviceUndefine(virConnectPtr conn, virNodeDevicePtr dev, unsigned int flags); int virNodeDeviceSetAutostart(virNodeDevicePtr dev, int autostart, unsigned int flags); int virNodeDeviceGetAutostart(virNodeDevicePtr dev, int *autostart, unsigned int flags); One API missed is: int virNodeDeviceCreate(virNodeDevicePtr dev, unsigned int flags); To create the vHBA. 2) Associate vHBA with domain XML There are two ways to attach a LUN to a domain: as an QEMU emulated device; or passthrough. Since passthrough a LUN is not supported in libvirt yet, let's focus on the emulated LUN at this stage. New attributes wwnn and wwpn are introduced to indicate the LUN behind the vHBA. E.g. disk type='block' device='disk' driver name='qemu' type='raw'/ source wwnn=2001001b32a9da4e wwpn=2101001b32a90004/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /disk Before the domain starting, we have to check if there is LUN assigned to the vHBA, error out if not. Using the stable path of LUN also works, e.g. source dev=/dev/disk/by-path/pci-\:00\:07.0-scsi-0\:0\:0\:0/ But the disadvantage is the user have to figure out the stable path himself; And we have to do checking of every stable path to see if it's behind a vHBA in migration Begin stage. Or an new XML tag for element source to indicate that it's behind a vHBA? such as: source dev=disk-by-path model=vport/ 3) Migration with vHBA One possible solution for migration with vHBA is to use one pair of WWNN WWPN on source host, one is using for domain, one is reserved for migration purpose. It requires the storage admin maps the same LUN to the two vHBAs when doing the masking and zoning. Is WWNN part of the migration? I mean, isn't WWNN normally associated w/ the underlying real vendor HBA and to have that also means the target of your migration has to match up that WWNN? No, it doesn't have to match the WWNN of HBA. Please look through the threads for the currrent agreement, it's much different with this proposal. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ivshmem server
On 2012年11月21日 02:16, Andrew Jones wrote: This mail is meant to get a discussion started. Please keep me on cc for the discussion, as I'm not subscribed to libvir-list. ivshmem is an implementation of an inter-VM communication channel. Support for this has been in qemu since v0.14.0 and libvirt patches have been recently posted[1]. What's still missing is the ivshmem server. The ivshmem server is needed when one would like to use interrupts with ivshmem. The server manages a set of eventfds to send/recv those interrupts. There is currently only one implementation of this server that I'm aware of, which is available from this git repo [2] in the ivshmem-server directory. My suggestion for libvirt is that this code be integrated into libvirt, rather than managed by libvirt, for the following reasons 1. libvirt should keep track of the socket path in order to build ivshmem's command line anyway. 2. the current ivshmem server code is ~300 lines, so it shouldn't be a large integration effort. 3. keeping ivhsmem server separate increases the package management that distributions need to do. afaik, it isn't currently packaged for any distribution. One concern I have with the git repo [2] is that I don't see any license for ivshmem-server. I've cc'ed Cam for his input. Add the missed CC. Drew [1] http://www.redhat.com/archives/libvir-list/2012-November/msg00731.html [2] git://gitorious.org/nahanni/guest-code.git -- 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] messed up with xml-files and configuration of a VM
On Tue, Nov 20, 2012 at 5:13 PM, Lentes, Bernd bernd.len...@helmholtz-muenchen.de wrote: first, i'm new to kvm. I'm running KVM on a sles 11 sp2, kernel 3.0.13-0.27-default. My guest is an Ubuntu 12.0.4 LTS 64bit. The guest has attached a CDROM, using an iso-file from a CIFS-Share. I detached it with the virtual machine manager (0.9.0). I don't see the cd-rom anymore in the virtual machine manager. But when i try to start the vm, it complains about the missing iso-file. Why ? I detached it. When i like to have a look in the xml-files of the guest, i found three ! One in /var/lib/kvm/images, one in /etc/libvirt/qemu and one in /etc/kvm/vm. Which one should i use to configure the vm ? In the one in /etc/libvirt/qemu the cifs-share isn't mentioned any longer, in the other two it is still. Is it possible to configure the vm editing one of the XML-files ? Or shall i use virsh ? Using virsh, does the vm has to be stopped or can i edit the configuration for a running vm ? Why three xml-files ? Why is detaching with the virtual machine manager not working ? Hi Bernd, This is a libvirt question, I have CCed the libvirt mailing list. Do not edit the XML files on disk. Instead, use virsh edit (to modify) and virsh dumpxml (to view). Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Porting Qemu driver to FreeBSD
Eric Blake wrote: On 11/20/2012 10:07 AM, Roman Bogorodskiy wrote: Hi, Recently I have picked up Sean's and Hiren's activities in porting Qemu driver on FreeBSD. At this point I have the basics working. Meaning that these things work: - Basic host commands (version, nodeinfo, etc) - Can perform basic operations on the VM (start, stop, destroy, etc) - Basic bridging (bridge creation/removing, adding/removing interfaces, setting addresses etc) Currently I keep FreeBSD specific info here: https://github.com/novel/fbsd-libvirt/blob/freebsd/README-freebsd Thanks for your porting efforts. Is this something where you have split it into a series of patches worth incorporating upstream? The code is available here: https://github.com/novel/fbsd-libvirt/tree/freebsd Current changes could be observed here: https://github.com/novel/fbsd-libvirt/compare/upstream_master...freebsd I would appreciate your thoughts and feedback on that topic. You're more likely to get feedback by posting actual patches here, rather than pointing us to a repo and expecting us to crawl through the differences. But here's hoping we can improve the code base as a result of your efforts. That sounds reasonable. I'll start working on creation of the sequential patches. Roman Bogorodskiy pgpeOudvUE92l.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list