- Devel - libvirt List Archives

[libvirt] RFCv2: virDomainSnapshotCreateXML enhancements
by Eric Blake 26 Aug '11

26 Aug '11

[BCC'ing those who have responded to earlier RFC's] I've posted previous RFCs for improving snapshot support: ideas on managing a subset of disks: https://www.redhat.com/archives/libvir-list/2011-May/msg00042.html ideas on managing snapshots of storage volumes not tied to a domain https://www.redhat.com/archives/libvir-list/2011-June/msg00761.html After re-reading the feedback received on those threads, I think I've settled on a pretty robust design for my first round of adding improvements to the management of snapshots tied to a domain, while leaving the door open for future extensions. Sorry this email is so long (I've had it open in my editor for more than 48 hours now as I keep improving it), but hopefully it is worth the effort to read. See the bottom if you want the shorter summary on the proposed changes. First, some definitions: ======================== disk snapshot: the state of a virtual disk used at a given time; once a snapshot exists, then it is possible to track a delta of changes that have happened since that time. internal disk snapshot: a disk snapshot where both the saved state and delta reside in the same file (possible with qcow2 and qed). If a disk image is not in use by qemu, this is possible via 'qemu-img snapshot -c'. external disk snapshot: a disk snapshot where the saved state is one file, and the delta is tracked in another file. For a disk image not in use by qemu, this can be done with qemu-img to create a new qcow2 file wrapping any type of existing file. Recent qemu has also learned the 'snapshot_blkdev' monitor command for creating external snapshots while qemu is using a disk, and the goal of this RFC is to expose that functionality from within existing libvirt APIs. saved state: all non-disk information used to resume a guest at the same state, assuming the disks did not change. With qemu, this is possible via migration to a file. checkpoint: a combination of saved state and a disk snapshot. With qemu, the 'savevm' monitor command creates a checkpoint using internal snapshots. It may also be possible to combine saved state and disk snapshots created while the guest is offline for a form of checkpointing, although this RFC focuses on disk snapshots created while the guest is running. snapshot: can be either 'disk snapshot' or 'checkpoint'; the rest of this email will attempt to use 'snapshot' where either form works, and a qualified term where no ambiguity is intended. Existing libvirt functionality ============================== The virDomainSnapshotCreateXML currently manages a hierarchy of "snapshots", although it is currently only used for "checkpoints", where every snapshot has a name and a possibly empty parent. The idea is that once a domain has a snapshot, there is always a current snapshot, and all new snapshots are created with a parent of a previously existing snapshot (although there are still some bugs to be fixed in managing the current snapshot over a libvirtd restart). It is possible to have disjoint hierarchies, if you delete a root snapshot that had more than one child (making both children become independent roots). The snapshot hierarchy is maintained by libvirt (in a typical installation, the files in /var/lib/libvirt/qemu/snapshot/<dom>/<name> track each named snapshot, using <domainsnapshot> XML); using additional metadata not present in the qcow2 internal snapshot format (that is, while qcow2 can maintain multiple snapshots, it does not maintain relations between them). Remember, the "current" snapshot is not the current machine state, but the snapshot that would become the parent if you create a new snapshot; perhaps we could have named it the "loaded" snapshot, but the API names are set in stone now. Libvirt also has APIs for listing all snapshots, querying the current snapshot, reverting back to the state of another snapshot, and deleting a snapshot. Deletion comes with a choice of deleting just that named version (removing one node in the hierarchy and re-parenting all children) or that tree of the hierarchy (that named version and all children). Since qemu checkpoints can currently only be created via internal disk snapshots, libvirt has not had to track any file name relationships - a single "snapshot" corresponds to a qcow2 snapshot name within all qcow2 disks associated to a domain; furthermore, snapshot creation was limited to domains where all modifiable disks were already in qcow2 format. However, these "checkpoints" could be created on both running domains (qemu savevm) or inactive domains (qemu-img snapshot -c), with the latter technically being a case of just internal disk snapshots. Libvirt currently has a bug in that it only saves <domain>/<uuid> rather than the full domain xml along with a checkpoint - if any devices are hot-plugged (or in the case of offline snapshots, if the domain configuration is changed) after a snapshot but before the revert, then things will most likely blow up due to the differences in devices in use by qemu vs. the devices expected by the snapshot. Reverting to a snapshot can also be considered as a form of data loss - you are discarding the disk changes and ram state that have happened since the last snapshot. To some degree, this is by design - the very nature of reverting to a snapshot implies throwing away changes; however, it may be nice to add a safety valve so that by default, reverting to a live checkpoint from an offline state works, but reverting from a running domain should require some confirmation that it is okay to throw away accumulated running state. Libvirt also currently has a limitation where snapshots are local to one host - the moment you migrate a snapshot to another host, you have lost access to all snapshot metadata. Proposed enhancements ===================== Note that these proposals merely add xml attribute and subelement extensions, as well as API flags, rather than creating any new API, which makes it a nice candidate for backporting the patch series based on this RFC into older releases as appropriate. Creation ++++++++ I propose reusing the virDomainSnapshotCreateXML API and <domainsnapshot> xml for both "checkpoints" and "disk snapshots", all maintained within a single hierarchy. That is, the parent of a disk snapshot can be a checkpoint or another disk snapshot, and the parent of a checkpoint can be another checkpoint or a disk snapshot. And, since I defined "snapshot" to mean either "checkpoint" or "disk snapshot", this single hierarchy of "snapshots" will still be valid once it is expanded to include more than just "checkpoints". Since libvirt already has to maintain additional metadata to track parent-child relationships between snapshots, it should not be hard to augment that XML to store additional information needed to track external disk snapshots. The default is that virDomainSnapshotCreateXML(,0) creates a checkpoint, while leaving qemu running; I propose two new flags to fine-tune things: virDomainSnapshotCreateXML(, VIR_DOMAIN_SNAPSHOT_CREATE_HALT) will create the checkpoint then halt the qemu process, and virDomainSnapshotCreateXML(, VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) will create a disk snapshot rather than a checkpoint (on qemu, by using a sequence including the new 'snapshot_blkdev' monitor command). Specifying both flags at once is a form of data loss (you are losing the ram state), and I suspect it to be rarely used, but since it may be worthwhile in testing whether a disk snapshot is truly crash-consistent, I won't refuse the combination. Other flags may be added in the future; I know of at least two features in qemu that may warrant some flags once they are stable: 1. a guest agent fsfreeze/fsthaw command will allow the guest to get the file system into a stable state prior to the snapshot, meaning that reverting to that snapshot can skip out on any fsck or journal replay actions. Of course, this is a best effort attempt since guest agent interaction is untrustworthy (comparable to memory ballooning - the guest may not support the agent or may intentionally send falsified responses over the agent), so the agent should only be used when explicitly requested - this would be done with a new flag VIR_DOMAIN_SNAPSHOT_CREATE_GUEST_FREEZE. 2. there is thought of adding a qemu monitor command to freeze just I/O to a particular subset of disks, rather than the current approach of having to pause all vcpus before doing a snapshot of multiple disks. Once that is added, libvirt should use the new monitor command by default, but for compatibility testing, it may be worth adding VIR_DOMAIN_SNAPSHOT_CREATE_VCPU_PAUSE to require a full vcpu pause instead of the faster iopause mechanism. My first xml change is that <domainsnapshot> will now always track the full <domain> xml (prior to any file modifications), normally as an output-only part of the snapshot (that is, a <domain> sublement of <domainsnapshot> will always be present in virDomainGetXMLDesc, but is generally ignored in virDomainSnapshotCreateXML - more on this below). This gives us the capability to use XML ABI compatibility checks (similar to those used in virDomainMigrate2, virDomainRestoreFlags, and virDomainSaveImageDefineXML). And, given that the full <domain> xml is now present in the snapshot metadata, this means that we need to add virDomainSnapshotGetXMLDesc(snap, VIR_DOMAIN_XML_SECURE), so that any security-sensitive data doesn't leak out to read-only connections. Right now, domain ABI compatibility is only checked for VIR_DOMAIN_XML_INACTIVE contents of xml; I'm thinking that the snapshot <domain> will always be the inactive version (sufficient for starting a new qemu), although I may end up changing my mind and storing the active version (when attempting to revert from live qemu to another live checkpoint, all while using a single qemu process, the ABI compatibility checking may need enhancements to discover differences not visible in inactive xml but fatally different between the active xml when using 'loadvm', but which not matter to virsh save/restore where a new qemu process is created every time). Next, we need a way to control which subset of disks is involved in a snapshot command. Previous mail has documented that for ESX, the decision can only be made at boot time - a disk can be persistent (involved in snapshots, and saves changes across domain boots); independent-persistent (is not involved in snapshots, but saves changes across domain boots); or independent-nonpersistent (is not involved in snapshots, and all changes during a domain run are discarded when the domain quits). In <domain> xml, I will represent this by two new optional attributes: <disk snapshot='no|external|internal' persistent='yes|no'>...</disk> For now, qemu will reject snapshot=internal (the snapshot_blkdev monitor command does not yet support it, although it was documented as a possible extension); I'm not sure whether ESX supports external, internal, or both. Likewise, both ESX and qemu will reject persistent=no unless snapshot=no is also specified or implied (it makes no sense to create a snapshot if you know the disk will be thrown away on next boot), but keeping the options orthogonal may prove useful for some future extension. If either option is omitted, the default for snapshot is 'no' if the disk is <shared> or <readonly> or persistent=no, and 'external' otherwise; and the default for persistent is 'yes' for all disks (domain_conf.h will have to represent nonpersistent=0 for easier coding with sane 0-initialized defaults, but no need to expose that ugly name in the xml). I'm not sure whether to reject an explicit persistent=no coupled with <readonly>, or just ignore it (if the disk is readonly, it can't change, so there is nothing to throw away after the domain quits). Creation of an external snapshot requires rewriting the active domain XML to reflect the new filename. While ESX can only select the subset of disks to snapshot at boot time, qemu can alter the selection at runtime. Therefore, I propose also modifying the <domainsnapshot> xml to take a new subelement <disks> to fine-tune which disks are involved in a snapshot. For now, a checkpoint must omit <disks> on virDomainSnapshotCreateXML input (that is, <disks> must only be present if the VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY is used, and checkpoints always cover full system state, and on qemu this checkpoint uses internal snapshots). Meanwhile, for disk snapshots, if the <disks> element is omitted, then one is automatically created using the attributes in the <domain> xml. For ESX, if the <disks> element is present, it must select the same disks as the <domain> xml. Offline checkpoints will continue to use <state>shutoff</state> in the xml output, while new disk snapshots will use <state>disk-snapshot</state> to indicate that the disk state was obtained from a running VM and might be only crash-consistent rather than stable. The <disks> element has an optional number of <disk> subelements; at most one per <disk> in the <devices> section of <domain>. Each <disk> element has a mandatory attribute name='name', which must match the <target dev='name'/> of the <domain> xml, as a way of getting 1:1 correspondence between domainsnapshot/disks/disk and domain/devices/disk while using names that should already be unique. Each <disk> also has an optional snapshot='no|internal|external' attribute, similar to the proposal for <domain>/<devices>/<disk>; if not provided, the attribute defaults to the one from the <domain>. If snapshot=external, then there may be an optional subelement <source file='path'/>, which gives the desired new file name. If external is requested, but the <source> subelement is not present, then libvirt will generate a suitable filename, probably by concatenating the existing name with the snapshot name, and remembering that the snapshot name is generated as a timestamp if not specified. Also, for external snapshots, the <disk> element may have an optional sub-element specifying the driver (useful for selecting qcow2 vs. qed in the qemu 'snapshot_blkdev' monitor command); again, this can normally be generated by default. Future extensions may include teaching qemu to allow coupling checkpoints with external snapshots by allowing a <disks> element even for checkpoints. (That is, while the initial implementation will always output <disks> for <state>disk-snapshot</state> and never output <disks> for <state>shutoff</state>, but this may not always hold in the future). Likewise, we may discover when implementing lvm or btrfs snapshots that additional subelements to each <disk> would be useful for specifying additional aspects for creating snapshots using that technology, where the omission of those subelements has a sane default state. libvirt can be taught to honor persistent=no for qemu by creating a qcow2 wrapper file prior to starting qemu, then tearing down that wrapper after the fact, although I'll probably leave that for later in my patch series. As an example, a valid input <domainsnapshot> for creation of a qemu disk snapshot would be: <domainsnapshot> <name>snapshot</name> <disks> <disk name='vda'/> <disk name='vdb' snapshot='no'/> <disk name='vdc' snapshot='external'> <source file='/path/to/new'/> </disk> </disks> </domainsnapshot> which requests that the <disk> matching the target dev=vda defer to the <domain> default for whether to snapshot (and if the domain default requires creating an external snapshot, then libvirt will create the new file name; this could also be specified by omitting the <disk name='vda'/> subelement altogether); the <disk> matching vdb is not snapshotted, and the <disk> matching vdc is involved in an external snapshot where the user specifies the new filename of /path/to/new. On dumpxml output, the output will be fully populated with the items generated by libvirt, and be displayed as: <domainsnapshot> <name>snapshot</name> <state>disk-snapshot</state> <parent> <name>prior</name> </parent> <creationTime>1312945292</creationTime> <domain>  ... <devices> <disk type='file' device='disk' snapshot='external'> <driver name='qemu' type='raw'/> <source file='/path/to/original'/> <target dev='vda' bus='virtio'/> </disk> ... </devices> </domain> <disks> <disk name='vda' snapshot='external'> <driver name='qemu' type='qcow2'/> <source file='/path/to/original.snapshot'> </disk> <disk name='vdb' snapshot='no'/> <disk name='vdc' snapshot='external'> <driver name='qemu' type='qcow2'/> <source file='/path/to/new'/> </disk> </disks> </domainsnapshot> And, if the user were to do 'virsh dumpxml' of the domain, they would now see the updated <disk> contents: <domain> ... <devices> <disk type='file' device='disk' snapshot='external'> <driver name='qemu' type='qcow2'/> <source file='/path/to/original.snapshot'/> <target dev='vda' bus='virtio'/> </disk> ... </devices> </domain> ++++++++++ Reverting When it comes to reverting to a snapshot, the only time it is possible to revert to a live image is if the snapshot is a "checkpoint" of a running or paused domain, because qemu must be able to restore the ram state. Reverting to any other snapshot (both the existing "checkpoint" of an offline image, which uses internal disk snapshots, and my new "disk snapshot" which uses external disk snapshots even though it was created against a running image), will revert the disks back to the named state, but default to leaving the guest in an offline state. Two new mutually exclusive flags will allow to both revert to snapshot disk state and affect the resulting qemu state; virDomainRevertToSnapshot(snap, VIR_DOMAIN_SNAPSHOT_REVERT_START) to run from the snapshot, and virDomainRevertToSnapshot(snap, VIR_DOMAIN_SNAPSHOT_REVERT_PAUSE) to create a new qemu process but leave it paused. If neither of these two flags is specified, then the default will be determined by the snapshot itself. These flags also allow overriding the running/paused aspect recorded in live checkpoints. Note that I am not proposing a flag for reverting to just the disk state of a live checkpoint; this is considered an uncommon operation, and can be accomplished in two steps by reverting to paused state to restore disk state followed by destroying the domain (but I can add a third mutually-exclusive flag VIR_DOMAIN_SNAPSHOT_REVERT_STOP if we decide that we really want this uncommon operation via a single API). Reverting from a stopped state is always allowed, even if the XML is incompatible, by basically rewriting the domain's xml definition. Meanwhile, reverting from an online VM to a live checkpoint has two flavors - if the XML is compatible, then the 'loadvm' monitor command can be used, and the qemu process remains alive. But if the XML has changed incompatibly since the checkpoint was created, then libvirt will refuse to do the revert unless it has permission to start a new qemu process, via another new flag: virDomainRevertToSnapshot(snap, VIR_DOMAIN_SNAPSHOT_REVERT_FORCE). The new REVERT_FORCE flag also provides a safety valve - reverting to a stopped state (whether an existing offline checkpoint, or a new disk snapshot) from a running VM will be rejected unless REVERT_FORCE is specified. For now, this includes the case of using the REVERT_START flag to revert to a disk snapshot and then start qemu - this is because qemu does not yet expose a way to safely revert to a disk snapshot from within the same qemu process. If, in the future, qemu gains support for undoing the effects of 'snapshot_blkdev' via monitor commands, then it may be possible to use REVERT_START without REVERT_FORCE and end up reusing the same qemu process while still reverting to the disk snapshot state, by using some of the same tricks as virDomainReboot to force the existing qemu process to boot from the new disk state. Of course, the new safety valve is a slight change in behavior - scripts that used to use 'virsh snapshot-revert' may now have to use 'virsh snapshot-revert --force' to do the same actions; for backwards compatibility, the virsh implementation should first try without the flag, and a new VIR_ERR_* code be introduced in order to let virsh distinguish between a new implementation that rejected the revert because _REVERT_FORCE was missing, and an old one that does not support _REVERT_FORCE in the first place. But this is not the first time that added safety valves have caused existing scripts to have to adapt - consider the case of 'virsh undefine' which could previously pass in a scenario where it now requires 'virsh undefine --managed-save'. For transient domains, it is not possible to make an offline checkpoint (since transient domains don't exist if they are not running or paused); transient domains must use REVERT_START or REVERT_PAUSE to revert to a disk snapshot. And given the above limitations about qemu, reverting to a disk snapshot will currently require REVERT_FORCE, since a new qemu process will necessarily be created. Just as creating an external disk snapshot rewrote the domain xml to match, reverting to an older snapshot will update the domain xml (it should be a bit more obvious now why the <domainsnapshot>/<domain>/<devices>/<disk> lists the old name, while <domainsnapshot>/<disks>/<disk> lists the new name). The other thing to be aware of is that with internal snapshots, qcow2 maintains a distinction between current state and a snapshot - that is, qcow2 is _always_ tracking a delta, and never modifies a named snapshot, even when you use 'qemu-img snapshot -a' to revert to different snapshot names. But with named files, the original file now becomes a read-only backing file to a new active file; if we revert to the original file, and make any modifications to it, the active file that was using it as backing will be corrupted. Therefore, the safest thing is to reject any attempt to revert to any snapshot (whether checkpoint or disk snapshot) that has an existing child snapshot consisting of an external disk snapshot. The metadata for each of these children can be deleted manually, but that requires quite a few API calls (learn how many children exist, get the list of children, and for each child, get its xml to see if that child has the target snapshot as a parent, and if so delete the snapshot). So as shorthand, virDomainRevertToSnapshot will be taught a new flag, VIR_DOMAIN_SNAPSHOT_REVERT_DELETE_CHILDREN, which first deletes any children of the snapshot about to be deleted prior to reverting to that particular child. And as long as reversion is learning how to do some snapshot deletion, it becomes possible to decide what to do with the qcow2 file that was created at the time of the disk snapshot. The default behavior for qemu will be to use qemu-img to recreate the qcow2 wrapper file as a 0-delta change against the original file, and keeping the domain xml tied to the wrapper name, but a new flag VIR_DOMAIN_SNAPSHOT_REVERT_DISCARD can be used to instead completely delete the qcow2 wrapper file, and update the domain xml back to the original filename. Deleting ++++++++ Deleting snapshots also needs some improvements. With checkpoints, the disk snapshot contents were internal snapshots, so no files had to be deleted. But with external disk snapshots, there are some choices to be made - when deleting a snapshot, should the two files be consolidated back into one or left separate, and if consolidation occurs, what should be the name of the new file. Right now, qemu supports consolidation only in one direction - the backing file can be consolidated into the new file by using the new blockpull API. In fact, the combination of disk snapshot and block pull can be used to implement local storage migration - create a disk snapshot with a local file as the new file around the remote file used as the snapshot, then use block pull to break the ties to the remote snapshot. But there is currently no way to make qemu save the contents of a new file back into its backing file and then swap back to the backing file as the live disk; also, while you can use block pull to break the relation between the snapshot and the live file, and then rename the live file back over the backing file name, there is no way to make qemu revert back to that file name short of doing the snapshot/blockpull algorithm twice; and the end result will be qcow2 even if the original file was raw. Also, if qemu ever adds support for merging back into a backing file, as well as a means to determine how dirty a qcow2 file is in relation to its backing file, there are some possible efficiency gains - if most blocks of a snapshot differ from the backing file, it is faster to use blockpull to pull in the remaining blocks from the backing file to the active file; whereas if most blocks of a snapshot are inherited from the backing file, it is more efficient to pull just the dirty blocks from the active file back into the backing file. Knowing whether the original file was qcow2 or some other format may also impact how to merge deltas from the new qcow2 file back into the original file. Additionally, having fine-tuned control over which of the two names to keep when consolidating a snapshot would require passing that information through xml, but the existing virDomainSnapshotDelete does not take an XML argument. For now, I propose that deleting an external disk snapshot will be required to leave both the snapshot and live disk image files intact (except for the special case of REVERT_DISCARD mentioned above that combines revert and delete into a single API); but I could see the feasibility of a future extension which adds a new XML <on_delete> subelement to <domainsnapshot>/<disks>/<disk> flags that specifies which of two files to consolidate into, as well as a flag VIR_DOMAIN_SNAPSHOT_DELETE_CONSOLIDATE which triggers libvirt to do the consolidation for any <on_delete> subelements in the snapshot being deleted (if the flag is omitted, the <on_delete> subelement is ignored and both files remain). The notion of deleting all children of a snapshot while keeping the snapshot itself (mentioned above under the revert use case) seems common enough that I will add a flag VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY; this flag implies VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, but leaves the target snapshot intact. Undefining ++++++++++ In one regards, undefining a domain that has snapshots is just as bad as undefining a domain with managed save state - since libvirt is maintaining metadata about snapshot hierarchies, leaving this metadata behind _will_ interfere with creation of a new domain by the same name. However, since both checkpoints and snapshots are stored in user-accessible disk images, and only the metadata is stored by libvirt, it should eventually be possible for the user to decide whether to discard the metadata but keep the snapshot contents intact in the disk images, or to discard both the metadata and the disk image snapshots. Meanwhile, I propose changing the default behavior of virDomainUndefine[Flags] to reject attempts to undefine a domain with any defined snapshots, and to add a new flag for virDomainUndefineFlags, virDomainUndefineFlags(,VIR_DOMAIN_UNDEFINE_SNAPSHOTS), to act as shorthand for calling virDomainSnapshotDelete for all snapshots tied to the domain. Note that this deletes the metadata, but not the underlying storage volumes. Migration +++++++++ The simplest solution to the fact that snapshot metadata is host-local is to make migration attempts fail if a domain has any associated snapshots. For a first cut patch, that is probably what I'll go with - it reduces libvirt functionality, but instantly plugs all the bugs that you can currently trigger by migrating a domain with snapshots. But we can do better. Right now, there is no way to inject the metadata associated with an already-existing snapshot, whether that snapshot is internal or external, and deleting internal snapshots always deletes the data as well as the metadata. But I already documented that external snapshots will keep both the new file and it's read-only original, in most cases, which means the data is preserved even when the snapshot is deleted. With a couple new flags, we can have virDomainSnapshotDelete(snap, VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) which removes libvirt's metadata, but still leaves all the data of the snapshot present (visible to qemu-img snapshot -l or via multiple file names); as well as virDomainSnapshotCreateXML(dom, xml, VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE), which says to add libvirt snapshot metadata corresponding to existing snapshots without doing anything to the current guest (no 'savevm' or 'snapshot_blkdev', although it may still make sense to do some sanity checks to see that the metadata being defined actually corresponds to an existing snapshot in 'qemu-img snapshot -l' or that an external snapshot file exists and has the correct backing file to the original name). Additionally, with these two tools in place, you can now make ABI-compatible tweaks to the <domain> xml stored in a snapshot metadata (similar to how 'virsh save-image-edit' can tweak a save image, such as changing the host name of a <disk>'s image to match what was done externally with qemu-img or other external tool). You can also make an extended protocol that first dumps all snapshot xml on the source, redefines those snapshots on the destination, then deletes the metadata on the source, all before migrating the domain itself (unfortunately, I don't think it can be wired into the cookies of migration protocol v3, as each <domainsnapshot> xml for each snapshot will be larger than the <domain> itself, and an arbitrary number of snapshots with lots of xml don't fit into a finite-sized cookie over rpc; ultimately, this may mean a migration protocol v4 that has an arbitrary number of handshakes between Begin on the source and Prepare on the dest in order to properly handle all the interchange - having a feature negotiation between client and host should be part of that interchange). Future proposals ================ I still want to add APIs to manage storage volume snapshots for storage volumes not associated with a current domain, as well as enhancing disk snapshots to operate on more than just qcow2 file formats (for example, lvm snapshots or btrfs copy-on-write clones). But I've already signed up for quite a bit of code changes in just this email, so that will have to come later. I hope that what I have designed here does not preclude extensibility to future additions - for example, <storagevolsnapshot> would be able to use a single <disk> sublement similar to the above <domainsnapshot>/<disks>/<disk> sublement for describing the relation between a disk and its backing file snapshot. Quick Summary ============= These are the changes I plan on making soon; I mentioned other possible future changes above that would depend on these being complete first, or which involve creation of new API. The following API patterns currently "succeed", but risk data loss or other bugs that can get libvirt into an inconsistent state; they will now fail by default: virDomainRevertToSnapshot to go from a running VM to a stopped checkpoint will now fail by default. Justification: stopping a running domain is a form of data loss. Mitigation: use VIR_DOMAIN_SNAPSHOT_REVERT_FORCE for old behavior. virDomainRevertToSnapshot to go from a running VM to a live checkpoint with an ABI-incompatible <domain> will now fail by default. Justification: qemu does not handle ABI incompatibilities, and even if the 'loadvm' may have succeeded, this generally resulted in fullscale guest corruption. Mitigation: use VIR_DOMAIN_SNAPSHOT_REVERT_FORCE to start a new qemu process that properly conforms to the snapshot's ABI. virDomainUndefine will now fail to undefine a domain with any snapshots. Justification: leaving behind libvirt metadata can corrupt future defines, comparable to recent managed save changes, plus it is a form of data loss. Mitigation: use virDomainUndefineFlags. virDomainUndefineFlags will now default to failing an undefine of a domain with any snapshots. Justification: leaving behind libvirt metadata can corrupt future defines, comparable to recent managed save changes, plus it is a form of data loss. Mitigation: separately delete all snapshots (or at least all snapshot metadata) first, or use VIR_DOMAIN_UNDEFINE_SNAPSHOTS. virDomainMigrate/virDomainMigrate2 will now default to fail if the source has any snapshots. Justification: metadata must be transferred along with the domain for the migration to be complete. Mitigation: until an improved migration protocol can automatically do the handshaking necessary to migrate all the snapshot metadata, a user can manually loop over each snapshot prior to migration, using virDomainSnapshotCreateXML with VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE on the destination, then virDomainSnapshotDelete with VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY on the source. Add the following XML: in <domain>/<devices>/<disk>: add optional attribute snapshot='no|internal|external' add optional attribute persistent='yes|no' in <domainsnapshot>: expand <domainsnapshot>/<domain> to be full domain, not just uuid add <state>disk-snapshot</state> add optional <disks>/<disk>, where each <disk> maps back to <domain>/<devices>/<disk> and controls how to do external disk snapshots Add the following flags to existing API: virDomainSnapshotCreateXML: VIR_DOMAIN_SNAPSHOT_CREATE_HALT VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE virDomainSnapshotGetXMLDesc VIR_DOMAIN_XML_SECURE virDomainRevertToSnapshot VIR_DOMAIN_SNAPSHOT_REVERT_START VIR_DOMAIN_SNAPSHOT_REVERT_PAUSE VIR_DOMAIN_SNAPSHOT_REVERT_FORCE VIR_DOMAIN_SNAPSHOT_REVERT_DELETE_CHILDREN VIR_DOMAIN_SNAPSHOT_REVERT_DISCARD virDomainSnapshotDelete VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY virDomainUndefineFlags VIR_DOMAIN_UNDEFINE_SNAPSHOTS -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

4 28

[libvirt] [test-API][PATCH] Add 2 functions to support get and set the scheduler parameters flag
by Nan Zhang 26 Aug '11

26 Aug '11

Add 2 functions into domain API get_sched_params_flags() set_sched_params_flags() --- lib/domainAPI.py | 26 +++++++++++++++++++++++++- 1 files changed, 25 insertions(+), 1 deletions(-) diff --git a/lib/domainAPI.py b/lib/domainAPI.py index 5667c20..a6efab7 100644 --- a/lib/domainAPI.py +++ b/lib/domainAPI.py @@ -523,6 +523,16 @@ class DomainAPI(object): code = e.get_error_code() raise exception.LibvirtAPI(message, code) + def get_sched_params_flags(self, domname, flags): + try: + dom_obj = self.get_domain_by_name(domname) + sched_params_flags = dom_obj.schedulerParametersFlags(flags) + return sched_params_flags + except libvirt.libvirtError, e: + message = e.get_error_message() + code = e.get_error_code() + raise exception.LibvirtAPI(message, code) + def set_sched_params(self, domname, params): try: dom_obj = self.get_domain_by_name(domname) @@ -533,6 +543,16 @@ class DomainAPI(object): code = e.get_error_code() raise exception.LibvirtAPI(message, code) + def set_sched_params_flags(self, domname, params, flags): + try: + dom_obj = self.get_domain_by_name(domname) + retval = dom_obj.setSchedulerParameters(params, flags) + return retval + except libvirt.libvirtError, e: + message = e.get_error_message() + code = e.get_error_code() + raise exception.LibvirtAPI(message, code) + def core_dump(self, domname, to, flags = 0): try: dom_obj = self.get_domain_by_name(domname) @@ -769,7 +789,6 @@ VIR_DOMAIN_SHUTDOWN = 4 VIR_DOMAIN_SHUTOFF = 5 VIR_DOMAIN_CRASHED = 6 - # virDomainMigrateFlags VIR_MIGRATE_LIVE = 1 VIR_MIGRATE_PEER2PEER = 2 @@ -780,3 +799,8 @@ VIR_MIGRATE_PAUSED = 32 VIR_MIGRATE_NON_SHARED_DISK = 64 VIR_MIGRATE_NON_SHARED_INC = 128 +# virDomainModificationImpact +VIR_DOMAIN_AFFECT_CURRENT = 0 +VIR_DOMAIN_AFFECT_LIVE = 1 +VIR_DOMAIN_AFFECT_CONFIG = 2 + -- 1.7.4.4

2 1

[libvirt] [PATCH] virsh: Clarify documentation of -d option
by Jiri Denemark 26 Aug '11

26 Aug '11

The default is 4, not 0. --- tools/virsh.pod | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index e17f309..9355d6c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -72,7 +72,8 @@ instead of the default connection. =item B<-d>, B<--debug> I<LEVEL> Enable debug messages at integer I<LEVEL> and above. I<LEVEL> can -range from 0 (default) to 4. +range from 0 to 4 (default). See the documentation of B<VIRSH_DEBUG> +environment variable for the description of each I<LEVEL>. =item B<-l>, B<--log> I<FILE> -- 1.7.6.1

2 2

[libvirt] [PATCH v3 0/3] Correctly label migration TCP socket
by Jiri Denemark 26 Aug '11

26 Aug '11

With current libvirt and qemu, migration is not working if SELinux is in enforcing mode, since the TCP socket we pass to qemu is not labeled in a way that would allow qemu to read from it. After this patchset, migration works even in enforcing mode. Jiri Denemark (3): security: Rename SetSocketLabel APIs to SetDaemonSocketLabel security: Introduce SetSocketLabel qemu: Correctly label migration TCP socket src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 5 +++- src/qemu/qemu_process.c | 3 +- src/security/security_dac.c | 11 +++++++++- src/security/security_driver.h | 3 ++ src/security/security_manager.c | 10 +++++++++ src/security/security_manager.h | 2 + src/security/security_nop.c | 7 ++++++ src/security/security_selinux.c | 42 +++++++++++++++++++++++++++++++++++++- src/security/security_stack.c | 17 +++++++++++++++ 10 files changed, 96 insertions(+), 5 deletions(-) -- 1.7.6.1

3 10

[libvirt] [test-API][PATCH v2] Add ownership_test.py test case
by Wayne Sun 26 Aug '11

26 Aug '11

* Save a domain to a file, check the ownership of the file after save and restore operation --- repos/domain/ownership_test.py | 302 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 302 insertions(+), 0 deletions(-) create mode 100644 repos/domain/ownership_test.py diff --git a/repos/domain/ownership_test.py b/repos/domain/ownership_test.py new file mode 100644 index 0000000..cba1424 --- /dev/null +++ b/repos/domain/ownership_test.py @@ -0,0 +1,302 @@ +#!/usr/bin/env python +"""Setting the dynamic_ownership in /etc/libvirt/qemu.conf, + check the ownership of saved domain file. Test could be on + local or root_squash nfs. + domain:ownership_test + guestname + #GUESTNAME# + dynamic_ownership + 0|1 + use_nfs + 0|1 + + use_nfs is a flag for decide using nfs or not +""" + +__author__ = 'Wayne Sun: gsun(a)redhat.com' +__date__ = 'Mon Jul 25, 2011' +__version__ = '0.1.0' +__credits__ = 'Copyright (C) 2011 Red Hat, Inc.' +__all__ = ['ownership_test'] + +import os +import re +import sys +import commands + +QEMU_CONF = "/etc/libvirt/qemu.conf" +SAVE_FILE = "/mnt/test.save" +TEMP_FILE = "/tmp/test.save" + +from utils.Python import utils + +def append_path(path): + """Append root path of package""" + if path in sys.path: + pass + else: + sys.path.append(path) + +from lib import connectAPI +from lib import domainAPI +from utils.Python import utils +from exception import LibvirtAPI + +pwd = os.getcwd() +result = re.search('(.*)libvirt-test-API', pwd) +append_path(result.group(0)) + +def return_close(conn, logger, ret): + conn.close() + logger.info("closed hypervisor connection") + return ret + +def check_params(params): + """Verify inputing parameter dictionary""" + logger = params['logger'] + keys = ['guestname', 'dynamic_ownership'] + for key in keys: + if key not in params: + logger.error("%s is required" %key) + return 1 + return 0 + +def nfs_setup(util, logger): + """setup nfs on localhost + """ + logger.info("set nfs service") + cmd = "echo /tmp *$rw,root_squash$ > /etc/exports" + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("failed to config nfs export") + return 1 + + logger.info("start nfs service") + cmd = "service nfs start" + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("failed to start nfs service") + return 1 + else: + for i in range(len(out)): + logger.info(out[i]) + + return 0 + +def chown_file(util, filepath, logger): + """touch a file and setting the chown + """ + if os.path.exists(filepath): + os.remove(filepath) + + touch_cmd = "touch %s" % filepath + logger.info(touch_cmd) + ret, out = util.exec_cmd(touch_cmd, shell=True) + if ret: + logger.error("failed to touch a new file") + logger.error(out[0]) + return 1 + + logger.info("set chown of %s as 107:107" % filepath) + chown_cmd = "chown 107:107 %s" % filepath + ret, out = util.exec_cmd(chown_cmd, shell=True) + if ret: + logger.error("failed to set the ownership of %s" % filepath) + return 1 + + logger.info("set %s mode as 644" % filepath) + cmd = "chmod 644 %s" % filepath + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("failed to set the mode of %s" % filepath) + return 1 + + return 0 + +def prepare_env(util, guestname, dynamic_ownership, use_nfs, logger): + """configure dynamic_ownership in /etc/libvirt/qemu.conf, + set chown of the file to save + """ + logger.info("set the dynamic ownership in %s as %s" % \ + (QEMU_CONF, dynamic_ownership)) + set_cmd = "echo dynamic_ownership = %s >> %s" % (dynamic_ownership, QEMU_CONF) + ret, out = util.exec_cmd(set_cmd, shell=True) + if ret: + logger.error("failed to set dynamic ownership") + + logger.info("restart libvirtd") + restart_cmd = "service libvirtd restart" + ret, out = util.exec_cmd(restart_cmd, shell=True) + if ret: + logger.error("failed to restart libvirtd") + return 1 + else: + for i in range(len(out)): + logger.info(out[i]) + + if use_nfs == '1': + ret = nfs_setup(util, logger) + if ret: + return 1 + + logger.info("mount the nfs path to /mnt") + mount_cmd = "mount -o vers=3 127.0.0.1:/tmp /mnt" + ret, out = util.exec_cmd(mount_cmd, shell=True) + if ret: + logger.error("Failed to mount the nfs path") + for i in range(len(out)): + logger.info(out[i]) + + filepath = TEMP_FILE + else: + filepath = SAVE_FILE + + ret = chown_file(util, filepath, logger) + if ret: + return 1 + + return 0 + +def ownership_get(logger): + """check the ownership of file""" + + statinfo = os.stat(SAVE_FILE) + uid = statinfo.st_uid + gid = statinfo.st_gid + + logger.info("the uid and gid of %s is %s:%s" %(SAVE_FILE, uid, gid)) + + return 0, uid, gid + +def ownership_test(params): + """Save a domain to a file, check the ownership of + the file after save and restore + """ + # Initiate and check parameters + params_check_result = check_params(params) + if params_check_result: + return 1 + + logger = params['logger'] + guestname = params['guestname'] + dynamic_ownership = params['dynamic_ownership'] + use_nfs = params['use_nfs'] + test_result = False + + util = utils.Utils() + + # set env + logger.info("prepare the environment") + ret = prepare_env(util, guestname, dynamic_ownership, use_nfs, logger) + if ret: + logger.error("failed to prepare the environment") + + # Connect to local hypervisor connection URI + uri = util.get_uri('127.0.0.1') + conn = connectAPI.ConnectAPI() + virconn = conn.open(uri) + + # save domain to the file + logger.info("save the domain to the file") + domobj = domainAPI.DomainAPI(virconn) + try: + domobj.save(guestname, SAVE_FILE) + except LibvirtAPI, e: + logger.error("API error message: %s, error code is %s" % \ + (e.response()['message'], e.response()['code'])) + logger.error("Error: fail to save %s domain" %guestname) + return return_close(conn, logger, 1) + + logger.info("check the ownership of %s after save" % SAVE_FILE) + ret, uid, gid = ownership_get(logger) + if use_nfs == '1': + if uid == 107 and gid == 107: + logger.info("As expected, the chown not change.") + test_result = True + else: + logger.error("The chown of %s is %s:%s, it's not as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + else: + if dynamic_ownership == '1': + if uid == 0 and gid == 0: + logger.info("As expected, the chown changed to root:root") + test_result = True + else: + logger.error("The chown of %s is %s:%s, it's not as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + elif dynamic_ownership == '0': + if uid == 107 and gid == 107: + logger.info("As expected, the chown not change.") + test_result = True + else: + logger.error("The chown of %s is %s:%s, it's not as expected" % \ + (SAVE_FILE, uid, gid)) + return return_close(conn, logger, 1) + else: + logger.error("wrong dynamic_ownership value %s" % dynamic_ownership) + return 1 + + + # restore domain from file + logger.info("restore the domain from the file") + try: + domobj.restore(guestname, SAVE_FILE) + logger.info("check the ownership of %s after restore" % SAVE_FILE) + ret, uid, gid = ownership_get(logger) + if uid == 107 and gid == 107: + logger.info("As expected, the chown not change.") + test_result = True + else: + logger.error("The chown of %s is %s:%s, not change back as expected" % \ + (SAVE_FILE, uid, gid)) + test_result = False + except LibvirtAPI, e: + logger.error("API error message: %s, error code is %s" % \ + (e.response()['message'], e.response()['code'])) + logger.error("Error: fail to restore %s domain" %guestname) + test_result = False + + if test_result: + return return_close(conn, logger, 0) + else: + return return_close(conn, logger, 1) + +def ownership_test_clean(params): + """clean testing environment""" + logger = params['logger'] + use_nfs = params['use_nfs'] + + util = utils.Utils() + + if use_nfs == '1': + logger.info("umount the nfs path") + umount_cmd = "umount /mnt" + ret, out = util.exec_cmd(umount_cmd, shell=True) + if ret: + logger.error("Failed to mount the nfs path") + for i in range(len(out)): + logger.error(out[i]) + + logger.info("stop nfs service") + cmd = "service nfs stop" + ret, out = util.exec_cmd(cmd, shell=True) + if ret: + logger.error("Failed to stop nfs service") + for i in range(len(out)): + logger.error(out[i]) + + logger.info("clear the exports file") + cmd = ">/etc/exports" + if ret: + logger.error("Failed to clear exports file") + + filepath = TEMP_FILE + else: + filepath = SAVE_FILE + + if os.path.exists(filepath): + logger.info("remove dump file from save %s" % filepath) + os.remove(filepath) + -- 1.7.1

2 1

[libvirt] [PATCH] Do not try to cancel non-existent migration on source
by Jiri Denemark 26 Aug '11

26 Aug '11

If migration failed on source daemon, the migration is automatically canceled by the daemon itself. Thus we don't need to call virDomainMigrateConfirm3(cancelled=1). Calling it doesn't cause any harm but the resulting error message printed in logs may confuse people. --- src/libvirt.c | 41 +++++++++++++++++++++++++---------------- 1 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index c8af3e1..256828c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4141,6 +4141,7 @@ virDomainMigrateVersion3(virDomainPtr domain, virErrorPtr orig_err = NULL; int cancelled = 1; unsigned long protection = 0; + bool notify_source = true; VIR_DOMAIN_DEBUG(domain, "dconn=%p xmlin=%s, flags=%lx, " "dname=%s, uri=%s, bandwidth=%lu", @@ -4221,8 +4222,13 @@ virDomainMigrateVersion3(virDomainPtr domain, uri, flags | protection, dname, bandwidth); /* Perform failed. Make sure Finish doesn't overwrite the error */ - if (ret < 0) + if (ret < 0) { orig_err = virSaveLastError(); + /* Perform failed so we don't need to call confirm to let source know + * about the failure. + */ + notify_source = false; + } /* If Perform returns < 0, then we need to cancel the VM * startup on the destination @@ -4265,22 +4271,25 @@ finish: confirm: /* - * If cancelled, then src VM will be restarted, else - * it will be killed - */ - VIR_DEBUG("Confirm3 %p ret=%d domain=%p", domain->conn, ret, domain); - VIR_FREE(cookiein); - cookiein = cookieout; - cookieinlen = cookieoutlen; - cookieout = NULL; - cookieoutlen = 0; - ret = domain->conn->driver->domainMigrateConfirm3 - (domain, cookiein, cookieinlen, - flags | protection, cancelled); - /* If Confirm3 returns -1, there's nothing more we can - * do, but fortunately worst case is that there is a - * domain left in 'paused' state on source. + * If cancelled, then src VM will be restarted, else it will be killed. + * Don't do this if migration failed on source and thus it was already + * cancelled there. */ + if (notify_source) { + VIR_DEBUG("Confirm3 %p ret=%d domain=%p", domain->conn, ret, domain); + VIR_FREE(cookiein); + cookiein = cookieout; + cookieinlen = cookieoutlen; + cookieout = NULL; + cookieoutlen = 0; + ret = domain->conn->driver->domainMigrateConfirm3 + (domain, cookiein, cookieinlen, + flags | protection, cancelled); + /* If Confirm3 returns -1, there's nothing more we can + * do, but fortunately worst case is that there is a + * domain left in 'paused' state on source. + */ + } done: if (orig_err) { -- 1.7.6

2 5

[libvirt] [PATCH] Ignore unused streams in virStreamAbort
by Jiri Denemark 26 Aug '11

26 Aug '11

When virStreamAbort is called on a stream that has not been used yet, quite confusing error is returned: "this function is not supported by the connection driver". Let's just ignore such streams as there's nothing to abort anyway. --- src/libvirt.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 256828c..72c47f8 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -13975,8 +13975,12 @@ int virStreamAbort(virStreamPtr stream) return -1; } - if (stream->driver && - stream->driver->streamAbort) { + if (!stream->driver) { + VIR_DEBUG("aborting unused stream"); + return 0; + } + + if (stream->driver->streamAbort) { int ret; ret = (stream->driver->streamAbort)(stream); if (ret < 0) -- 1.7.6

2 2

[libvirt] [test-API][PATCH] add "imagetype" argument for guest installation testcases
by Guannan Ren 26 Aug '11

26 Aug '11

Add disk format argument to installation test scripts. With them, we can test snapshot testing. The following is the conf file sample. domain:install_linux_cdrom guestname test-api-guest ... imagetype qcow2 ... domain:shutdown guestname test-api-guest snapshot:internal_create guestname test-api-guest --- repos/domain/install_linux_cdrom.py | 23 ++++++++++----- repos/domain/install_linux_net.py | 50 +++++++++++++++++---------------- repos/domain/install_windows_cdrom.py | 23 ++++++++++----- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 7e8fee9..8d21797 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -8,6 +8,7 @@ optional arguments: memory vcpu disksize + imagetype imagepath hdmodel nicmodel @@ -70,6 +71,7 @@ def usage(): optional arguments: memory vcpu disksize + imagetype imagepath hdmodel nicmodel @@ -86,7 +88,7 @@ def check_params(params): mandatory_args = ['guestname', 'guesttype', 'guestos', 'guestarch'] optional_args = ['memory', 'vcpu', 'disksize', 'imagepath', 'hdmodel', 'nicmodel', 'macaddr', 'ifacetype', 'source', 'type', - 'volumepath'] + 'volumepath', 'imagetype'] for arg in mandatory_args: if arg not in params_given.keys(): @@ -255,7 +257,7 @@ def install_linux_cdrom(params): logger.debug("the uri to connect is %s" % uri) if params.has_key('imagepath') and not params.has_key('volumepath'): - imgfullpath = os.join.path(params.get('imagepath'), guestname) + imgfullpath = os.path.join(params.get('imagepath'), guestname) elif not params.has_key('imagepath') and not params.has_key('volumepath'): if hypervisor == 'xen': @@ -280,13 +282,18 @@ def install_linux_cdrom(params): else: seeksize = '10' - logger.info("the size of disk image is %sG" % (seeksize)) - shell_disk_dd = "dd if=/dev/zero of=%s bs=1 count=1 seek=%sG" % \ - (imgfullpath, seeksize) - logger.debug("the commands line of creating disk images is '%s'" % - shell_disk_dd) + if params.has_key('imagetype'): + imagetype = params.get('imagetype') + else: + imagetype = 'raw' + + logger.info("create disk image with size %sG, format %s" % (seeksize, imagetype)) + disk_create = "qemu-img create -f %s %s %sG" % \ + (imagetype, imgfullpath, seeksize) + logger.debug("the commands line of creating disk images is '%s'" % \ + disk_create) - (status, message) = commands.getstatusoutput(shell_disk_dd) + (status, message) = commands.getstatusoutput(disk_create) if status != 0: logger.debug(message) diff --git a/repos/domain/install_linux_net.py b/repos/domain/install_linux_net.py index 21ae378..1b0470e 100644 --- a/repos/domain/install_linux_net.py +++ b/repos/domain/install_linux_net.py @@ -9,6 +9,7 @@ optional arguments: memory vcpu disksize + imagetype imagepath hdmodel nicmodel @@ -72,6 +73,7 @@ def usage(): optional arguments: memory vcpu disksize + imagetype imagepath hdmodel nicmodel @@ -88,7 +90,8 @@ def check_params(params): 'guestarch','netmethod'] optional_args = ['memory', 'vcpu', 'disksize', 'imagepath', - 'hdmodel', 'nicmodel', 'ifacetype', 'source', 'type'] + 'hdmodel', 'nicmodel', 'ifacetype', + 'imagetype', 'source', 'type'] for arg in mandatory_args: if arg not in params_given.keys(): @@ -233,7 +236,7 @@ def install_linux_net(params): logger.debug("the uri to connect is %s" % uri) if params.has_key('imagepath'): - fullimagepath = os.join.path(params.get('imagepath'), guestname) + fullimagepath = os.path.join(params.get('imagepath'), guestname) else: if hypervisor == 'xen': fullimagepath = os.path.join('/var/lib/xen/images', guestname) @@ -246,29 +249,28 @@ def install_linux_net(params): fullimagepath) if params.has_key('disksize'): - logger.info("the size of disk image is %sG" % (params.get('disksize'))) - shell_disk_dd = "dd if=/dev/zero of=%s bs=1 count=1 seek=%sG" % \ - (fullimagepath, params.get('disksize')) - logger.debug("the commands line of creating disk images is '%s'" % - shell_disk_dd) - - (status, message) = commands.getstatusoutput(shell_disk_dd) - if status != 0: - logger.debug(message) - else: - logger.info("creating disk images file is successful.") + seeksize = params.get('disksize') else: - logger.info("the size of disk image is 10G") - shell_disk_dd = "dd if=/dev/zero of=%s bs=1 count=1 seek=10G" % \ - fullimagepath - logger.debug("the commands line of creating disk images is '%s'" % - shell_disk_dd) - - (status, message) = commands.getstatusoutput(shell_disk_dd) - if status != 0: - logger.debug(message) - else: - logger.info("creating disk images file is successful.") + seeksize = '10' + + if params.has_key('imagetype'): + imagetype = params.get('imagetype') + else: + imagetype = 'raw' + + logger.info("create disk image with size %sG, format %s" % (seeksize, imagetype)) + disk_create = "qemu-img create -f %s %s %sG" % \ + (imagetype, fullimagepath, seeksize) + logger.debug("the commands line of creating disk images is '%s'" % \ + disk_create) + + (status, message) = commands.getstatusoutput(disk_create) + + if status != 0: + logger.debug(message) + else: + logger.info("creating disk images file is successful.") + logger.info("get system environment information") envfile = os.path.join(homepath, 'env.cfg') diff --git a/repos/domain/install_windows_cdrom.py b/repos/domain/install_windows_cdrom.py index 2ea0ee7..9cf9e3b 100644 --- a/repos/domain/install_windows_cdrom.py +++ b/repos/domain/install_windows_cdrom.py @@ -8,6 +8,7 @@ optional arguments: memory vcpu disksize + imagetype imagepath hdmodel nicmodel @@ -68,6 +69,7 @@ def usage(): optional arguments: memory vcpu disksize + imagetype imagepath hdmodel nicmodel @@ -89,7 +91,7 @@ def check_params(params): mandatory_args = ['guestname', 'guesttype', 'guestos', 'guestarch'] optional_args = ['memory', 'vcpu', 'disksize', 'imagepath', 'hdmodel', 'nicmodel', 'macaddr', 'ifacetype', 'source', 'type', - 'volumepath'] + 'volumepath', 'imagetype'] for arg in mandatory_args: if arg not in params_given.keys(): @@ -294,7 +296,7 @@ def install_windows_cdrom(params): logger.debug("the uri to connect is %s" % uri) if params.has_key('imagepath') and not params.has_key('volumepath'): - imgfullpath = os.join.path(params.get('imagepath'), guestname) + imgfullpath = os..path.join(params.get('imagepath'), guestname) elif not params.has_key('imagepath') and not params.has_key('volumepath'): if hypervisor == 'xen': imgfullpath = os.path.join('/var/lib/xen/images', guestname) @@ -318,13 +320,18 @@ def install_windows_cdrom(params): else: seeksize = '20' - logger.info("the size of disk image is %sG" % (seeksize)) - shell_disk_dd = "dd if=/dev/zero of=%s bs=1 count=1 seek=%sG" % \ - (imgfullpath, seeksize) - logger.debug("the commands line of creating disk images is '%s'" % - shell_disk_dd) + if params.has_key('imagetype'): + imagetype = params.get('imagetype') + else: + imagetype = 'raw' + + logger.info("create disk image with size %sG, format %s" % (seeksize, imagetype)) + disk_create = "qemu-img create -f %s %s %sG" % \ + (imagetype, imgfullpath, seeksize) + logger.debug("the commands line of creating disk images is '%s'" % \ + disk_create) - (status, message) = commands.getstatusoutput(shell_disk_dd) + (status, message) = commands.getstatusoutput(disk_create) if status != 0: logger.debug(message) -- 1.7.1

2 1

[libvirt] [test-API][PATCH] Fix the missing ret variable problem
by Wayne Sun 26 Aug '11

26 Aug '11

--- repos/remoteAccess/tls_setup.py | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/repos/remoteAccess/tls_setup.py b/repos/remoteAccess/tls_setup.py index 80d6b42..4e7f24e 100644 --- a/repos/remoteAccess/tls_setup.py +++ b/repos/remoteAccess/tls_setup.py @@ -343,6 +343,7 @@ def request_credentials(credentials, user_data): def hypervisor_connecting_test(uri, auth_tls, username, password, logger, expected_result): """ connect remote server """ + ret = 0 try: conn = connectAPI.ConnectAPI() if auth_tls == 'none': @@ -355,6 +356,7 @@ def hypervisor_connecting_test(uri, auth_tls, username, except LibvirtAPI, e: logger.error("API error message: %s, error code is %s" % \ (e.response()['message'], e.response()['code'])) + ret = 1 conn.close() -- 1.7.1

2 1

[libvirt] problem of escaped scancodes
by Lai Jiangshan 26 Aug '11

26 Aug '11

Hi, Daniel P. Berrange, Our user found that some keycode can not be handled well when they try to map other keycode to xt_kbd keycode, when xt_kbd keycode is an escaped scancode. The xt_kbd keycode http://git.gnome.org/browse/gtk-vnc/plain/src/keymaps.csv are come from x86_keycodes[] of linux/drivers/char/keyboard.c. And x86_keycodes[] are: static const unsigned short x86_keycodes[256] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84,118, 86, 87, 88,115,120,119,121,112,123, 92, 284,285,309, 0,312, 91,327,328,329,331,333,335,336,337,338,339, 367,288,302,304,350, 89,334,326,267,126,268,269,125,347,348,349, 360,261,262,263,268,376,100,101,321,316,373,286,289,102,351,355, 103,104,105,275,287,279,258,106,274,107,294,364,358,363,362,361, 291,108,381,281,290,272,292,305,280, 99,112,257,306,359,113,114, 264,117,271,374,379,265,266, 93, 94, 95, 85,259,375,260, 90,116, 377,109,111,277,278,282,283,295,296,297,299,300,301,293,303,307, 308,310,313,314,315,317,318,319,320,357,322,323,324,325,276,330, 332,340,365,342,343,344,345,346,356,270,341,368,369,370,371,372 }; There is no keycode in range [128, 256] in this table, which means all escaped scancode "e0 ??" are encoded as "0x100 | ??" in this table. Example, LeftCtrl is 0x1d, and RightCtrl is escaped: "e0 1d", RightCtrl is encoded as "0x100 | 0x1d"(=0x11d=285) in this table. The code also tell me the same information: code = x86_keycodes[keycode]; if (!code) return -1; if (code & 0x100) put_queue(vc, 0xe0); put_queue(vc, (code & 0x7f) | up_flag); But some other place, escaped scancode "e0 ??" are encoded as "0x80 | ??", this encoding is more commonly used, and qemu uses this encoding, RightCtrl is encoded as "0x80 | 0x1d"(=0x9d=157) in qemu: { 0x9d, "ctrl_r" }, keycode = keycodes[i]; if (keycode & 0x80) kbd_put_keycode(0xe0); kbd_put_keycode(keycode & 0x7f); The problem: keymaps.csv uses the first encoding while qemu uses the second one, it causes "virsh send-key" command can't work when it sends escaped scancodes. Example: When do "virsh send-key domain KEY_RIGHTCTRL", qemu will receive keycode=285 which is not expected. So I suggest to change keymaps.csv. Covert the old KEY_RIGHTCTRL,97,RightControl,0x3e,,,88,,285,228,VK_RCONTROL,0xa3,0x65,0x65 to new KEY_RIGHTCTRL,97,RightControl,0x3e,,,88,,157,228,VK_RCONTROL,0xa3,0x65,0x65 (ditto for other escaped scancodes) Or just add the new line to keymaps.csv followed by the old line, and make 285 and 157 can work at the same time. Thought? Thanks, Lai.

3 4

[libvirt] XML files
by Derek 26 Aug '11

26 Aug '11

Hi Folks, I'm running KVM on Debian squeeze. I have some requirements on a new VM to provide customised BIOS vendor/product strings, from my experience so far this is accomplish with the sysinfo/smbios support. So I have added these to the XML file for my VM. I found 'virsh edit' is the recommended way to make changes to a VM configuration, I find the XML file is not referenced at boot, it is only saved to with 'virsh dumpxml'. 'virsh edit' will update both areas - hypervisor and XML file. Unfortunately the changes I make for smbios and sysinfo (as below) while editing don't appear to be accepted when saving with 'virsh edit'. After saving I receive "Domain HMC XML configuration edited." but the XML file remains unmodified. I wonder if they are perhaps not supported with virsh edit yet? <sysinfo type='smbios'> <bios> <entry name='vendor'>xxx xxx</entry> <entry name='version'>xxx</entry> </bios> <system> <entry name='product'>xxx</entry> <entry name='version'>xxx</entry> <entry name='manufacturer'>xxx xxx</entry> </system> </sysinfo> <os> <type arch='x86_64' machine='pc-0.12'>hvm</type> <boot dev='cdrom'/> <smbios mode='sysinfo'/> </os> Thanks in advance, Derek

2 2

[libvirt] [libvirt-virshcmdref 01/14] update documentation for command attach-device
by Hu Tao 26 Aug '11

26 Aug '11

--- source/attach-device.xml | 143 ++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 137 insertions(+), 6 deletions(-) diff --git a/source/attach-device.xml b/source/attach-device.xml index efa46f0..18e3262 100644 --- a/source/attach-device.xml +++ b/source/attach-device.xml @@ -5,20 +5,151 @@ <description> <text> - Attach device from an XML file + Attach a device from an XML file </text> </description> - <options /> + <options> + <parameter requirement="required"> + <keyword requirement="optional">--domain</keyword> + <value type="string" requirement="required">domain</value> + <description> + <text> + domain name, id or uuid + </text> + <text> + "--domain" itself is optional + </text> + </description> + </parameter> + <parameter requirement="required"> + <keyword requirement="optional">--file</keyword> + <value type="string" requirement="required">file</value> + <description> + <text> + the XML file describing the device to be attached + </text> + <text> + "--file" itself is optional + </text> + </description> + </parameter> + <parameter requirement="optional"> + <keyword requirement="required">--persistent</keyword> + <description> + <text> + with this option, the XML file of domain will be updated + </text> + </description> + </parameter> + </options> <availability from="0.2.3" /> <notes /> - <examples type="usage" /> + <examples type="usage"> + <example> + <terminal> +virsh # <bold>attach-device</bold> <value>example-domain</value> <value>/tmp/new-nic.xml</value></terminal> + <text> + Attaches a NIC which is defined by <value>/tmp/new-nic.xml</value> to domain <value>example-domain</value>. + </text> + </example> + </examples> - <examples type="fullcontext" /> - - <reference type="seealso" /> + <examples type="fullcontext"> + <example> + <text> + In this example we will attach a NIC to a running domain which doesn't have a NIC. + The domain is defined as: + </text> + <terminal> +<domain type='kvm'> + <name>example-domain</name> + <uuid>6853c36f-af03-4968-a31d-1f3a9c3f699a</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.12'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/local/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' cache='writeback'/> + <source file='/mnt/data/libvirt-images/vm2.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + <video> + <model type='cirrus' vram='9216' heads='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain></terminal> + <text> + Bring up the domain. From the output of <value>lspci</value> we can see that there is no NIC + present. + </text> + <terminal>00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02) +00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] +00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] +00:01.2 USB Controller: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] (rev 01) +00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03) +00:02.0 VGA compatible controller: Cirrus Logic GD 5446 +00:03.0 RAM memory: Red Hat, Inc Virtio memory balloon +00:04.0 SCSI storage controller: Red Hat, Inc Virtio block device</terminal> + <text> + Now we attach a NIC to the domain: + </text> + <terminal>virsh # <bold>attach-device</bold> <value>example-domain</value> <value>/tmp/new-nic.xml</value></terminal> + <text> + the content of <value>/tmp/new-nic.xml</value> is: + </text> + <terminal> +<interface type='network'> + <source network='default'/> +</interface></terminal> + <text> + We can see that the attached NIC is listed by <value>lspci</value>: + </text> + <terminal>00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02) +00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] +00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] +00:01.2 USB Controller: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] (rev 01) +00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03) +00:02.0 VGA compatible controller: Cirrus Logic GD 5446 +00:03.0 RAM memory: Red Hat, Inc Virtio memory balloon +00:04.0 SCSI storage controller: Red Hat, Inc Virtio block device +00:05.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (rev 20)</terminal> + </example> + </examples> + <reference type="seealso"> + <item> + <link type="internal" href="detach-device" /> + <name> + detach-device + </name> + <description> + Detach a device from an XML file + </description> + </item> + </reference> </command> -- 1.7.3.1

3 31

[libvirt] [PATCH 1/3] virt-manager: Add controller model in details
by Marc-André Lureau 25 Aug '11

25 Aug '11

--- src/virtManager/details.py | 5 +++++ src/vmm-details.glade | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 0 deletions(-) diff --git a/src/virtManager/details.py b/src/virtManager/details.py index 1fb7d3a..fd6cc3a 100644 --- a/src/virtManager/details.py +++ b/src/virtManager/details.py @@ -3019,7 +3019,12 @@ class vmmDetails(vmmGObjectUI): return type_label = virtinst.VirtualController.pretty_type(dev.type) + model_label = dev.model + if not model_label: + model_label = _("Default") + self.widget("controller-type").set_text(type_label) + self.widget("controller-model").set_text(model_label) def refresh_filesystem_page(self): dev = self.get_hw_selection(HW_LIST_COL_DEVICE) diff --git a/src/vmm-details.glade b/src/vmm-details.glade index 0e2d631..8ab386b 100644 --- a/src/vmm-details.glade +++ b/src/vmm-details.glade @@ -5553,6 +5553,7 @@ I/O:</property> <widget class="GtkTable" id="table14"> <property name="visible">True</property> <property name="n_columns">2</property> + <property name="n_rows">2</property> <property name="column_spacing">6</property> <property name="row_spacing">6</property> <child> @@ -5577,6 +5578,34 @@ I/O:</property> <property name="y_options">GTK_FILL</property> </packing> </child> + <child> + <widget class="GtkLabel" id="mlabel48"> + <property name="visible">True</property> + <property name="label" translatable="yes">Model:</property> + </widget> + <packing> + <property name="x_options">GTK_FILL</property> + <property name="y_options">GTK_FILL</property> + <property name="left_attach">0</property> + <property name="right_attach">1</property> + <property name="top_attach">1</property> + <property name="bottom_attach">2</property> + </packing> + </child> + <child> + <widget class="GtkLabel" id="controller-model"> + <property name="visible">True</property> + <property name="label">foo</property> + </widget> + <packing> + <property name="left_attach">1</property> + <property name="right_attach">2</property> + <property name="top_attach">1</property> + <property name="bottom_attach">2</property> + <property name="x_options">GTK_FILL</property> + <property name="y_options">GTK_FILL</property> + </packing> + </child> </widget> </child> </widget> -- 1.7.6

1 2

[libvirt] [PATCH 0/4] python-virtinst USB improvements
by Marc-André Lureau 25 Aug '11

25 Aug '11

Hi, Here are a few patches that go with the libvirt series to improve USB support. It adds --controller and --usbredir, as well as providing some new API for virt-manager. The last patch default USB controller to ich9/usb2 on Windows 7 guest. More OS probably support it, since it's been around since early 2007. Marc-André Lureau (4): Fix typo s/type/managed Add advanced --controller support, augmenting VirtualController Add --usbredir server:port device RFC: Default to ICH9 USB2 controller for Win7 man/en/virt-clone.1 | 6 ++- man/en/virt-image.1 | 45 +++++++-------- man/en/virt-install.1 | 52 ++++++++++++++++- man/en/virt-install.pod.in | 59 ++++++++++++++++++- tests/cli-test-xml/compare/many-devices.xml | 30 ++++++++++ tests/clitest.py | 50 ++++++++++++++++ tests/utils.py | 8 +- tests/xmlparse-xml/change-controllers-in.xml | 4 + tests/xmlparse-xml/change-controllers-out.xml | 4 + tests/xmlparse.py | 9 +++ virt-install | 2 + virtinst/Guest.py | 26 ++++++++ virtinst/VirtualController.py | 78 +++++++++++++++++++++++-- virtinst/VirtualDevice.py | 49 +++++++++++++++- virtinst/VirtualHostDevice.py | 67 +++++++++++++++++++++- virtinst/XMLBuilderDomain.py | 10 +++ virtinst/__init__.py | 5 +- virtinst/cli.py | 78 +++++++++++++++++++++++++ virtinst/osdict.py | 3 + virtinst/support.py | 6 ++ 20 files changed, 547 insertions(+), 44 deletions(-) -- 1.7.6

1 4

[libvirt] [PATCH 1/2] qemu: avoid dead store in doPeer2PeerMigrate3
by ajia＠redhat.com 25 Aug '11

25 Aug '11

* src/qemu/qemu_migration.c: avoid dead 'ret' assignment and silence clang warning. Detected by ccc-analyzer: CC libvirt_driver_qemu_la-qemu_migration.lo qemu/qemu_migration.c:2046:5: warning: Value stored to 'ret' is never read ret = qemuMigrationConfirm(driver, sconn, vm, ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ --- src/qemu/qemu_migration.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5510493..469bb4a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2050,6 +2050,9 @@ finish: * do, but fortunately worst case is that there is a * domain left in 'paused' state on source. */ + if (ret < 0) + VIR_WARN("Guest %s probably left in 'paused' state on source", + vm->def->name); cleanup: if (ddomain) { -- 1.7.1

4 5

[libvirt] [PATCH] qemu: Correctly label migration TCP socket passed to qemu
by Jiri Denemark 25 Aug '11

25 Aug '11

--- src/qemu/qemu_migration.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a84faf6..d787a09 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1484,9 +1484,12 @@ qemuMigrationRun(struct qemud_driver *driver, case MIGRATION_DEST_FD: if (spec->fwdType != MIGRATION_FWD_DIRECT) fd = spec->dest.fd.local; - ret = qemuMonitorMigrateToFd(priv->mon, migrate_flags, - spec->dest.fd.qemu); - VIR_FORCE_CLOSE(spec->dest.fd.qemu); + if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm, + spec->dest.fd.qemu) == 0) { + ret = qemuMonitorMigrateToFd(priv->mon, migrate_flags, + spec->dest.fd.qemu); + VIR_FORCE_CLOSE(spec->dest.fd.qemu); + } break; } qemuDomainObjExitMonitorWithDriver(driver, vm); -- 1.7.6.1

4 7

[libvirt] [PATCH v2] remote: Refuse connecting to remote socket
by Michal Privoznik 25 Aug '11

25 Aug '11

If users wants to connect to remote unix socket, e.g. 'qemu+unix://<remote>/system' currently the <remote> part is ignored, ending up connecting to localhost. Connecting to remote socket is not supported and user should have used TLS/TCP/SSH instead. --- src/remote/remote_driver.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 28f333b..563ccf4 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -325,9 +325,16 @@ doRemoteOpen (virConnectPtr conn, } else { if (STRCASEEQ (transport_str, "tls")) transport = trans_tls; - else if (STRCASEEQ (transport_str, "unix")) - transport = trans_unix; - else if (STRCASEEQ (transport_str, "ssh")) + else if (STRCASEEQ (transport_str, "unix")) { + if (conn->uri->server) { + remoteError(VIR_ERR_INVALID_ARG, "%s", + _("rempte_open: using unix socket and " + "remote server is not supported.")); + return VIR_DRV_OPEN_ERROR; + } else { + transport = trans_unix; + } + } else if (STRCASEEQ (transport_str, "ssh")) transport = trans_ssh; else if (STRCASEEQ (transport_str, "ext")) transport = trans_ext; -- 1.7.3.4

2 3

[libvirt] [PATCH] Fix command test wrt gnutls initialize & fix debugging
by Daniel P. Berrange 25 Aug '11

25 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The VIR_TEST_DEBUG and VIR_TEST_VERBOSE env vars did not work because we replaced 'environ' with 'newenv'. Simply calling virTestGetDebug/Verbose() before replacing the 'environ' ensures we have processed the env variables. The gnutls initialization code opens /dev/urandom and keeps that FD around for later use. We have code which kills off FDs 3-5 to avoid interfereing with our test case. Move the virInitialize call before this point, so it kills off the gnutls /dev/urandom FD which is irrelevant for testing purposes * tests/commandtest.c: Fix test debugging & make it robust against opened FDs --- tests/commandtest.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index 4a798fc..dd6c248 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -773,6 +773,13 @@ mymain(void) setpgid(0, 0); setsid(); + + /* Prime the debug/verbose settings from the env vars, + * since we're about to reset 'environ' */ + virTestGetDebug(); + virTestGetVerbose(); + + virInitialize(); /* Kill off any inherited fds that might interfere with our * testing. */ fd = 3; @@ -782,8 +789,6 @@ mymain(void) fd = 5; VIR_FORCE_CLOSE(fd); - virInitialize(); - environ = (char **)newenv; # define DO_TEST(NAME) \ -- 1.7.6

2 1

[libvirt] ALERT Virtualization Automatic Builder
by berrange＠redhat.com 25 Aug '11

25 Aug '11

Overall status: failed Start date: Thu Aug 25 2011 Start time: 11:05:02 UTC / 07:05:02 EDT Build counter: 1314270302 Build timestamp: 1314270302 URL: http://builder.virt-tools.org/index.html Module: libvirt Status: failed URL: http://builder.virt-tools.org/module-libvirt.html

1 0

[libvirt] Libvirt 0.9.4rc2 + qemu 0.15 : shutdown domain
by Radek Hladik 24 Aug '11

24 Aug '11

Hi again, while experimenting with new libvirt and qemu I discovered different behavior than with my previous versions. When I shutdown machine with shutdown (virsh) or virDomainShutdown (via php) the domain is shutdown but it ends up paused instead of destroyed. The OS is shut down, CPU probably stopped and I can connect to VNC. I would suppose its because of the new reboot feature and the -no-shutdown option for qemu. I am curious whether it is a bug or expected behavior. It would make perfect sense to be able to "semi"shutdown machine like this. But I was not able to reboot the machine from this state by any means. The best I could do was to resume (unpause) the CPU. Radek

1 0

[libvirt] Libvirt 0.9.4rc2 + qemu 0.15 VNC/TLS not working
by Radek Hladik 24 Aug '11

24 Aug '11

Hi all, I am trying new versions of software for my libvirt/qemu farm. I upgraded libvirt to 0.9.3 and I had problems with loading certificates for libvirt itself (probably GNUTLS related). Upgrade to 0.9.4rc2 solved the problem. Now I am trying to upgrade qemu (from 0.12.5) to 0.15. I compiled the SRPM from rawhide for Fedora 13 (OS on my farms) and I copied the binary to one farm, so I can choose qemu version by <emulator> element in guest XML. I had problem with booting off virtio disk, but upgrade of seabios to 0.6.2 solved it (I had to make a wrapper for qemu to be able to add -bios option though). But main issue that I am not able to solve is that VNC is not using TLS. I've found out these: * qemu 0.12.5 is working without problems * there is no error in libvirt or qemu machine log * If I copy the command from libvirt log, remove the network and monitor definition and start it under root, TLS works as expected * the vnc definition on command line is quite simple: -vnc 0.0.0.0:0,password,tls,x509=/etc/pki/libvirt/pki-vnc -k en-us I am thinking whether there is not a problem with monitor setting something after the machine starts. Libvirt does the same with password, so maybe it does something with TLS. Unfortunately I do not know whether there is any option how to debug the monitor libvirt<->qemu communication. And I have one other question. Is there any way how to interact directly with monitor? Qemu has new guest agent that seems to provide some nice functions and expose them via monitor and I would like to test it. Radek

3 3

[libvirt] Automated builds of libvirt
by Daniel P. Berrange 24 Aug '11

24 Aug '11

You may have seen references to the libvirt automated builder with various patches sent to the list. Previously this was running on one of my own private machines, but I've now got it in a dedicated virtual machine, and have it rsyncing its results to a publically visible site: http://builder.virt-tools.org/ It is currently configure to run every 4 hours to build latest GIT master for all the apps listed. It won't rebuild if the source code hasn't changed since the last build, unless one of the dependant modules has changed. If it finds a build failure, it will spam libvir-list with an ALERT message The build logs for the failure can be found on the libvirt status page http://builder.virt-tools.org/module-libvirt.html It will only alert on the first failed build. Until a later build attempt is successfull it will avoid sending further alerts. This should avoid any out of control spamming of the list. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

1 0

[libvirt] [PATCH 2/3] qemu: When reserving all functions on a slot don't reserve function 0 on the next slot.
by Shradha Shah 24 Aug '11

24 Aug '11

Signed-off-by: Steve Hodgson <shodgson(a)solarflare.com> --- src/qemu/qemu_command.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dbfc7d9..12adcc0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -878,7 +878,7 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, { int function; - for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + for (function = 0; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { if (qemuDomainPCIAddressReserveFunction(addrs, slot, function) < 0) goto cleanup; } @@ -956,7 +956,7 @@ int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot) dev.addr.pci.bus = 0; dev.addr.pci.slot = slot; - for (*function = 0; *function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; (*function)++) { + for (*function = 0; *function < QEMU_PCI_ADDRESS_LAST_FUNCTION; (*function)++) { addr = qemuPCIAddressAsString(&dev); if (!addr) return -1; -- 1.7.4.4

2 1

[libvirt] [PATCH 1/3] pci: Fix callers of pciDeviceListSteal() when there's more than one device
by Shradha Shah 24 Aug '11

24 Aug '11

pciDeviceListSteal(pcidevs, dev) removes dev from pcidevs reducing the length of pcidevs, so moving onto what was the next dev is wrong. Instead callers should pop entry 0 repeatedly until pcidevs is empty. Signed-off-by: Steve Hodgson <shodgson(a)solarflare.com> Signed-off-by: Shradha Shah <sshah(a)solarflare.com> --- src/qemu/qemu_hostdev.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 7f5ad51..6f77717 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -74,7 +74,6 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, virDomainDefPtr def) { pciDeviceList *pcidevs; - int i; int ret = -1; if (!def->nhostdevs) @@ -83,8 +82,8 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) return -1; - for (i = 0; i < pciDeviceListCount(pcidevs); i++) { - pciDevice *dev = pciDeviceListGet(pcidevs, i); + while (pciDeviceListCount(pcidevs) > 0) { + pciDevice *dev = pciDeviceListGet(pcidevs, 0); pciDeviceListSteal(pcidevs, dev); if (pciDeviceListAdd(driver->activePciHostdevs, dev) < 0) { pciFreeDevice(dev); @@ -152,8 +151,8 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, } /* Now steal all the devices from pcidevs */ - for (i = 0; i < pciDeviceListCount(pcidevs); i++) { - pciDevice *dev = pciDeviceListGet(pcidevs, i); + while (pciDeviceListCount(pcidevs) > 0) { + pciDevice *dev = pciDeviceListGet(pcidevs, 0); pciDeviceListSteal(pcidevs, dev); } @@ -164,8 +163,8 @@ inactivedevs: /* Only steal all the devices from driver->activePciHostdevs. We will * free them in pciDeviceListFree(). */ - for (i = 0; i < pciDeviceListCount(pcidevs); i++) { - pciDevice *dev = pciDeviceListGet(pcidevs, i); + while (pciDeviceListCount(pcidevs) > 0) { + pciDevice *dev = pciDeviceListGet(pcidevs, 0); pciDeviceListSteal(driver->activePciHostdevs, dev); } -- 1.7.4.4

2 1

[libvirt] [PATCH v2] daemon: Move TLS initialization to virInitialize
by Michal Privoznik 24 Aug '11

24 Aug '11

My previous patch 74c75671331d284e1f777f9692b72e9737520bf0 introduced a regression by removing TLS initialization from client. --- diff to v1: -remove virNetTLSDeinit() daemon/libvirtd.c | 2 -- src/libvirt.c | 3 +++ src/rpc/virnettlscontext.c | 8 -------- src/rpc/virnettlscontext.h | 1 - 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5969a82..423c3d7 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1516,7 +1516,6 @@ int main(int argc, char **argv) { virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START, 0, "start", NULL); - virNetTLSInit(); if (daemonSetupNetworking(srv, config, sock_file, sock_file_ro, ipsock, privileged) < 0) { @@ -1555,7 +1554,6 @@ cleanup: virNetServerProgramFree(qemuProgram); virNetServerClose(srv); virNetServerFree(srv); - virNetTLSDeinit(); if (statuswrite != -1) { if (ret != 0) { /* Tell parent of daemon what failed */ diff --git a/src/libvirt.c b/src/libvirt.c index c8af3e1..fdf3923 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -40,6 +40,7 @@ #include "memory.h" #include "configmake.h" #include "intprops.h" +#include "rpc/virnettlscontext.h" #ifndef WITH_DRIVER_MODULES # ifdef WITH_TEST @@ -409,6 +410,8 @@ virInitialize(void) virLogSetFromEnv(); + virNetTLSInit(); + VIR_DEBUG("register drivers"); #if HAVE_WINSOCK2_H diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 8482eaf..e901b63 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1410,11 +1410,3 @@ void virNetTLSInit(void) { gnutls_global_init(); } - -/* - * See virNetTLSInit - */ -void virNetTLSDeinit(void) -{ - gnutls_global_deinit(); -} diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 99f31b9..fdfce6d 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -31,7 +31,6 @@ typedef virNetTLSSession *virNetTLSSessionPtr; void virNetTLSInit(void); -void virNetTLSDeinit(void); virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath, bool tryUserPkiPath, -- 1.7.3.4

2 1

[libvirt] [PATCH] util: only fchown newly created files in virFileOpenAs
by Laine Stump 24 Aug '11

24 Aug '11

virFileOpenAs takes desired uid:gid as arguments, and not only uses them for a fork/setuid/setgid when retrying failed open operations, but additionally always forces the opened file to be owned by the given uid:gid. One example of the problems this causes is that, when restoring a domain from a file that is owned by the qemu user, opening the file chowns it to root. if dynamic_ownership=1 this is coincidentally expected, but if dynamic_ownership=0, no existing file should ever have its ownership changed. This patch adds an extra check before calling fchown() - it only does it if O_CREAT was passed to virFileOpenAs() in the openflags. --- src/util/util.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index b278165..9556bdb 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -697,6 +697,7 @@ virFileOpenAsNoFork(const char *path, int openflags, mode_t mode, goto error; } if (((st.st_uid != uid) || (st.st_gid != gid)) + && (openflags & O_CREAT) && (fchown(fd, uid, gid) < 0)) { ret = -errno; virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), -- 1.7.3.4

2 1

[libvirt] [PATCH 3/3] If VIR_MIGRATE_PERSIST_DEF is set then make the live configuration transient
by Shradha Shah 24 Aug '11

24 Aug '11

Make the migrated VM config as transient. Ensures transient hotplug operations do not persist past shutdown. Currently the running VM config is transient but that of a migrated VM is persistent. Signed-off-by: Steve Hodgson <shodgson(a)solarflare.com> Signed-off-by: Shradha Shah <sshah(a)solarflare.com> --- src/qemu/qemu_migration.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a84faf6..aa458c3 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2509,6 +2509,9 @@ qemuMigrationFinish(struct qemud_driver *driver, goto endjob; } + /* Make the VM transient again to match qemuProcessStart */ + virDomainObjSetDefTransient(driver->caps, vm, true); + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, newVM ? -- 1.7.4.4

1 0

[libvirt] [PATCH] daemon: Move TLS initialization to virInitialize
by Michal Privoznik 24 Aug '11

24 Aug '11

My previous patch 74c75671331d284e1f777f9692b72e9737520bf0 introduced a regression by removing TLS initialization from client. --- daemon/libvirtd.c | 1 - src/libvirt.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5969a82..8f04a99 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1516,7 +1516,6 @@ int main(int argc, char **argv) { virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START, 0, "start", NULL); - virNetTLSInit(); if (daemonSetupNetworking(srv, config, sock_file, sock_file_ro, ipsock, privileged) < 0) { diff --git a/src/libvirt.c b/src/libvirt.c index c8af3e1..fdf3923 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -40,6 +40,7 @@ #include "memory.h" #include "configmake.h" #include "intprops.h" +#include "rpc/virnettlscontext.h" #ifndef WITH_DRIVER_MODULES # ifdef WITH_TEST @@ -409,6 +410,8 @@ virInitialize(void) virLogSetFromEnv(); + virNetTLSInit(); + VIR_DEBUG("register drivers"); #if HAVE_WINSOCK2_H -- 1.7.3.4

3 4

[libvirt] [PATCHv2 00/26] revised round of snapshot patches
by Eric Blake 24 Aug '11

24 Aug '11

Many, but not all, of these patches have been previously submitted. I've rebased the series, and added in some additional patches throughout. It still isn't complete, and could probably use more testing after about patch 20 or so, but I'd like to get reviews on some of these to start getting these off the queue. Eric Blake (26): virsh: concatenate qemu-monitor-command arguments snapshot: better event when reverting qemu to paused snapshot snapshot: improve reverting to qemu paused snapshots snapshot: properly revert qemu to offline snapshots snapshot: one less point of failure in qemu snapshot: only pass snapshot to qemu command line when reverting snapshot: track current snapshot across restarts snapshot: allow deletion of just snapshot metadata snapshot: avoid crash when deleting qemu snapshots snapshot: simplify acting on just children snapshot: let qemu discard only snapshot metadata snapshot: identify which snapshots have metadata snapshot: identify qemu snapshot roots snapshot: prevent stranding snapshot data on domain destruction snapshot: refactor some qemu code snapshot: cache qemu-img location snapshot: support new undefine flags in qemu snapshot: teach virsh about new undefine flags snapshot: prevent migration from stranding snapshot data snapshot: refactor domain xml output snapshot: allow full domain xml in snapshot snapshot: update rng to support full domain in xml snapshot: store qemu domain details in xml snapshot: add 2 attributes to domain xml for disks snapshot: reject transient disks where code is not ready snapshot: wire up new qemu monitor command docs/formatdomain.html.in | 40 +- docs/formatsnapshot.html.in | 45 +- docs/schemas/Makefile.am | 1 + docs/schemas/domain.rng | 2555 +----------------------- docs/schemas/{domain.rng => domaincommon.rng} | 25 +- docs/schemas/domainsnapshot.rng | 19 +- include/libvirt/libvirt.h.in | 40 +- src/conf/domain_conf.c | 503 ++++-- src/conf/domain_conf.h | 41 +- src/esx/esx_driver.c | 35 +- src/libvirt.c | 91 +- src/libvirt_private.syms | 4 + src/libxl/libxl_conf.c | 5 + src/qemu/qemu_command.c | 12 +- src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 839 +++++--- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 24 + src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 33 + src/qemu/qemu_monitor_json.h | 4 + src/qemu/qemu_monitor_text.c | 40 + src/qemu/qemu_monitor_text.h | 4 + src/qemu/qemu_process.c | 10 +- src/qemu/qemu_process.h | 1 + src/vbox/vbox_tmpl.c | 40 +- src/xenxs/xen_sxpr.c | 5 + src/xenxs/xen_xm.c | 5 + tests/domainsnapshotxml2xmlout/full_domain.xml | 35 + tools/virsh.c | 419 ++++- tools/virsh.pod | 42 +- 31 files changed, 1780 insertions(+), 3144 deletions(-) copy docs/schemas/{domain.rng => domaincommon.rng} (99%) create mode 100644 tests/domainsnapshotxml2xmlout/full_domain.xml -- 1.7.4.4

3 71

[libvirt] RFC: managing "pci passthrough" usage of sriov VFs via a new network forward type
by Laine Stump 24 Aug '11

24 Aug '11

For some reason beyond my comprehension, the designers of SRIOV ethernet cards decided that the virtual functions (VF) of the card (each VF corresponds to an ethernet device, e.g. "eth10") should each be given a new+different+random MAC address each time the hardware is rebooted. Normally, udev keeps a persistent table that associates each known MAC address with an ethernet device name - any time an ethernet device with a previously-unknown MAC address is found, a new device name is allocated ("eth11", etc) and the newly found MAC address is associated with that device name. When an ethernet device is an SRIOV VF, though, udev doesn't persist the MAC address, so at each boot a device is found with a new MAC addres, but the device name from the previous boot is "unused" so magically the device ends up with the same name even though the MAC address has changed. When this device is assigned to a guest via PCI passthrough, though, the guest doesn't have the necessary information to realize that it's actually an SRIOV VF, so the guest's udev persists the MAC address - on the first boot of host+guest, the guest will see it has, e.g., mac address 11:22:33:44:55:66 and udev will add an entry to its persistent table remembering that 11:22:33:44:55:66="eth0". If the host reboots, though, the VF will get a new MAC address, and when the guest boots, it will see a new MAC address (e.g. "66:55:44:33:22:11") and think that there's a different card, so it will create a new device (and a new udev entry - 66:55:44:33:22:11="eth1"). This will repeat each time the host reboots, with the obvious undesired consequences. This makes using SRIOV VFs via PCI passthrough very unpalatable. The problem can be solved by setting the MAC address of the ethernet device prior to assigning it to the guest, but of course the <hostdev> element used to assign PCI devices to guests has no place to specify a MAC address (and I'm not sure it would be appropriate to add something that function-specific to <hostdev>). Dave Allan and I have discussed a different possible method of eliminating this problem (using a new forward type for libvirt networks) that I've outlined below. Please let me know what you think - is this reasonable in general? If so, what about the details? If not, any counter-proposals to solve the problem? Providing Predictable/Configurable MAC Addresses for SRIOV VFs used via PCI Passthrough: 1) <network> will have a new forward type='hardware'. When forward type='hardware', a pool of ethernet interfaces can be specified, just as for the forward types "bridge", "vepa", "private", and "passthrough". At this point, that's the only thing that I've determined is needed in the network definition. 2) In a domain's <interface> definition, when type='network', if the network has a forward type='hardware', the domain code will request an unused ethernet device from the network driver, then do the following: 3) save the ethernet device name in interface/actual so that it can be easily retrieved if libvirtd is restarted 4) Set the MAC address of the given ethernet device according to the domain <interface> config. 5) Use the NodeDevice API to learn all the necessary PCI domain/slot/bus/function and add a (non-persisting) <hostdev> element to the guest's config before starting it up. 6) When the guest is eventually destroyed, the ethernet device will be free'd back to the network pool for use by another guest. One problem this doesn't solve is that when a guest is migrated, the PCI info for the allocated ethernet device on the destination host will almost surely be different. Is there any provision for dealing with this in the device passthrough code? If not, then migration will still not be possible. Although I realize that many people are predisposed to not like the idea of PCI passthrough of ethernet devices (including me), it seems that it's going to be used, so we may as well provide the management tools to do it in a sane manner.

4 7

[libvirt] [PATCH] libvirt: log all flags parameters
by Eric Blake 24 Aug '11

24 Aug '11

I was testing a virsh patch, and wanted to see if I had passed the flags I thought. But with LIBVIRT_DEBUG in the environment, I just saw: 14:24:52.359: 15022: debug : virDomainSnapshotNum:15586 : dom=0xc9c180, (VM: name=rhel_6-64, uuid=48f8e8e7-e14f-0e14-02f0-ce71997bdcab), including a trailing space. This fixes the issues. * src/libvirt.c: Log flag parameters, even if currently unused. (VIR_DOMAIN_DEBUG_0): Drop trailing comma in log. (VIR_DOMAIN_DEBUG_1): Split guts into... (VIR_DOMAIN_DEBUG_2): ...new macro. --- src/libvirt.c | 58 +++++++++++++++++++++++++++++++------------------------- 1 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index c8af3e1..a1197e0 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -333,10 +333,14 @@ static struct gcry_thread_cbs virTLSThreadImpl = { /* Internal use only, when VIR_DOMAIN_DEBUG has one argument. */ #define VIR_DOMAIN_DEBUG_0(dom) \ - VIR_DOMAIN_DEBUG_1(dom, "%s", "") + VIR_DOMAIN_DEBUG_2(dom, "%s", "") /* Internal use only, when VIR_DOMAIN_DEBUG has three or more arguments. */ -#define VIR_DOMAIN_DEBUG_1(dom, fmt, ...) \ +#define VIR_DOMAIN_DEBUG_1(dom, fmt, ...) \ + VIR_DOMAIN_DEBUG_2(dom, ", " fmt, __VA_ARGS__) + +/* Internal use only, with final format. */ +#define VIR_DOMAIN_DEBUG_2(dom, fmt, ...) \ do { \ char _uuidstr[VIR_UUID_STRING_BUFLEN]; \ const char *_domname = NULL; \ @@ -348,7 +352,7 @@ static struct gcry_thread_cbs virTLSThreadImpl = { _domname = (dom)->name; \ } \ \ - VIR_DEBUG("dom=%p, (VM: name=%s, uuid=%s), " fmt, \ + VIR_DEBUG("dom=%p, (VM: name=%s, uuid=%s)" fmt, \ dom, NULLSTR(_domname), _uuidstr, __VA_ARGS__); \ } while (0) @@ -1615,7 +1619,7 @@ error: char * virConnectGetSysinfo (virConnectPtr conn, unsigned int flags) { - VIR_DEBUG("conn=%p", conn); + VIR_DEBUG("conn=%p, flags=%x", conn, flags); virResetLastError(); @@ -2098,7 +2102,7 @@ virDomainDestroyFlags(virDomainPtr domain, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain); + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); virResetLastError(); @@ -2386,7 +2390,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "to=%s dxml=%s flags=%x", + VIR_DOMAIN_DEBUG(domain, "to=%s, dxml=%s, flags=%x", to, NULLSTR(dxml), flags); virResetLastError(); @@ -2816,7 +2820,7 @@ virDomainScreenshot(virDomainPtr domain, unsigned int screen, unsigned int flags) { - VIR_DOMAIN_DEBUG(domain, "stream=%p flags=%x", stream, flags); + VIR_DOMAIN_DEBUG(domain, "stream=%p, flags=%x", stream, flags); virResetLastError(); @@ -3292,7 +3296,7 @@ virDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "memory=%lu flags=%x", memory, flags); + VIR_DOMAIN_DEBUG(domain, "memory=%lu, flags=%x", memory, flags); virResetLastError(); @@ -3635,7 +3639,8 @@ virDomainGetState(virDomainPtr domain, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "state=%p, reason=%p", state, reason); + VIR_DOMAIN_DEBUG(domain, "state=%p, reason=%p, flags=%x", + state, reason, flags); virResetLastError(); @@ -3682,7 +3687,7 @@ virDomainGetControlInfo(virDomainPtr domain, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "info=%p", info); + VIR_DOMAIN_DEBUG(domain, "info=%p, flags=%x", info, flags); virResetLastError(); @@ -3791,7 +3796,7 @@ char *virConnectDomainXMLFromNative(virConnectPtr conn, const char *nativeConfig, unsigned int flags) { - VIR_DEBUG("conn=%p, format=%s config=%s flags=%x", + VIR_DEBUG("conn=%p, format=%s, config=%s, flags=%x", conn, nativeFormat, nativeConfig, flags); virResetLastError(); @@ -3844,7 +3849,7 @@ char *virConnectDomainXMLToNative(virConnectPtr conn, const char *domainXml, unsigned int flags) { - VIR_DEBUG("conn=%p, format=%s xml=%s flags=%x", + VIR_DEBUG("conn=%p, format=%s, xml=%s, flags=%x", conn, nativeFormat, domainXml, flags); virResetLastError(); @@ -3913,7 +3918,7 @@ virDomainMigrateVersion1 (virDomainPtr domain, int cookielen = 0, ret; virDomainInfo info; VIR_DOMAIN_DEBUG(domain, - "dconn=%p flags=%lx, dname=%s, uri=%s, bandwidth=%lu", + "dconn=%p, flags=%lx, dname=%s, uri=%s, bandwidth=%lu", dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); ret = virDomainGetInfo (domain, &info); @@ -4006,7 +4011,7 @@ virDomainMigrateVersion2 (virDomainPtr domain, virErrorPtr orig_err = NULL; int cancelled; VIR_DOMAIN_DEBUG(domain, - "dconn=%p flags=%lx, dname=%s, uri=%s, bandwidth=%lu", + "dconn=%p, flags=%lx, dname=%s, uri=%s, bandwidth=%lu", dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); /* Prepare the migration. @@ -6458,7 +6463,8 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, virConnectPtr conn; unsigned long nr_stats_ret = 0; - VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u", stats, nr_stats); + VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u, flags=%x", + stats, nr_stats, flags); virResetLastError(); @@ -6539,8 +6545,8 @@ virDomainBlockPeek (virDomainPtr dom, { virConnectPtr conn; - VIR_DOMAIN_DEBUG(dom, "path=%s, offset=%lld, size=%zi, buffer=%p", - path, offset, size, buffer); + VIR_DOMAIN_DEBUG(dom, "path=%s, offset=%lld, size=%zi, buffer=%p, flags=%x", + path, offset, size, buffer, flags); virResetLastError(); @@ -6712,7 +6718,7 @@ virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoP { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain, "info=%p flags=%x", info, flags); + VIR_DOMAIN_DEBUG(domain, "info=%p, flags=%x", info, flags); virResetLastError(); @@ -10343,7 +10349,7 @@ virStoragePoolCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { - VIR_DEBUG("conn=%p, xmlDesc=%s", conn, xmlDesc); + VIR_DEBUG("conn=%p, xmlDesc=%s, flags=%x", conn, xmlDesc, flags); virResetLastError(); @@ -10392,7 +10398,7 @@ virStoragePoolDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) { - VIR_DEBUG("conn=%p, xml=%s", conn, xml); + VIR_DEBUG("conn=%p, xml=%s, flags=%x", conn, xml, flags); virResetLastError(); @@ -10527,7 +10533,7 @@ virStoragePoolCreate(virStoragePoolPtr pool, unsigned int flags) { virConnectPtr conn; - VIR_DEBUG("pool=%p", pool); + VIR_DEBUG("pool=%p, flags=%x", pool, flags); virResetLastError(); @@ -10731,7 +10737,7 @@ virStoragePoolRefresh(virStoragePoolPtr pool, unsigned int flags) { virConnectPtr conn; - VIR_DEBUG("pool=%p flags=%x", pool, flags); + VIR_DEBUG("pool=%p, flags=%x", pool, flags); virResetLastError(); @@ -11491,7 +11497,7 @@ virStorageVolDownload(virStorageVolPtr vol, unsigned long long length, unsigned int flags) { - VIR_DEBUG("vol=%p stream=%p offset=%llu length=%llu flags=%x", + VIR_DEBUG("vol=%p, stream=%p, offset=%llu, length=%llu, flags=%x", vol, stream, offset, length, flags); virResetLastError(); @@ -11562,7 +11568,7 @@ virStorageVolUpload(virStorageVolPtr vol, unsigned long long length, unsigned int flags) { - VIR_DEBUG("vol=%p stream=%p offset=%llu length=%llu flags=%x", + VIR_DEBUG("vol=%p, stream=%p, offset=%llu, length=%llu, flags=%x", vol, stream, offset, length, flags); virResetLastError(); @@ -12043,7 +12049,7 @@ error: */ char *virNodeDeviceGetXMLDesc(virNodeDevicePtr dev, unsigned int flags) { - VIR_DEBUG("dev=%p, conn=%p", dev, dev ? dev->conn : NULL); + VIR_DEBUG("dev=%p, conn=%p, flags=%x", dev, dev ? dev->conn : NULL, flags); virResetLastError(); @@ -15537,7 +15543,7 @@ virDomainSnapshotNum(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - VIR_DOMAIN_DEBUG(domain); + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); virResetLastError(); -- 1.7.4.4

2 2

Re: [libvirt] [virt-tools-list] [PATCH] virt-manager: Show if the shut off vm has a saved image
by Eric Blake 24 Aug '11

24 Aug '11

[adding libvirt, for a virsh idea] On 08/01/2011 02:19 PM, Cole Robinson wrote: > On 07/30/2011 04:15 PM, Miklos Vajna wrote: >> Hi, >> >> So far in virt-manager one had to click on a shut off vm in the >> "Shutoff" state to see if it has a saved image: once the item is >> selected then the "play" icon will be "Run" or "Restore". This means >> that in case one has a lot of VMs, it's hard to get an overview of what >> VM has a saved image and what not. >> >> The attached patch changes the "Shutoff" status message to "Saved" in >> case the vm has a saved image. >> > > Good idea, I've pushed your patch: Awesome idea; in fact, I think 'virsh list --all' should probably also be enhanced to do something similar (show "saved" instead of "shutoff", and perhaps requiring a new --managed-save flag so as not to break back-compat). Note that virsh 0.9.4 is gaining 'virsh dominfo' as a way of querying whether a managed save image exists (previously virsh had no way to expose that information), but 'virsh dominfo' on a single domain at a time is much slower than doing it in bulk in 'virsh list'. Alas, we've missed the deadline for getting a patch like that into libvirt 0.9.4. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

3 5

[libvirt] [PATCH] qemu: Allow graceful domain destroy
by Michal Privoznik 24 Aug '11

24 Aug '11

This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable. The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition. --- include/libvirt/libvirt.h.in | 11 ++- src/qemu/qemu_driver.c | 132 +++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 22 +++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 20 ++++++ src/qemu/qemu_monitor_text.h | 1 + tools/virsh.c | 8 ++- 9 files changed, 198 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa29fb6..fa98b8d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,13 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ -/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ + +typedef enum { + VIR_DOMAIN_DESTROY_MONITOR = 1 << 0, /* In hypervisors supporting this, + try to send 'quit' command prior + to killing hypervisor */ +} virDomainDestroyFlagsValues; + virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..6585cb4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1564,6 +1564,42 @@ cleanup: return ret; } +struct qemuDomainDestroyHelperData { + struct qemud_driver *driver; + virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv; + virMutex lock; + virCond cond; + bool guard; +}; + +static void +qemuDomainDestroyHelper(void *opaque) +{ + struct qemuDomainDestroyHelperData *data = opaque; + struct qemud_driver *driver = data->driver; + virDomainObjPtr vm = data->vm; + qemuDomainObjPrivatePtr priv = data->priv; + + /* Job was already obtained by caller */ + qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuMonitorQuit(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + + /* To avoid loosing signal, we need to remember + * we tried to send one, but nobody was waiting + * for it. + */ + virMutexLock(&data->lock); + if (data->guard) { + virCondSignal(&data->cond); + } else { + data->guard = true; + } + virMutexUnlock(&data->lock); +} + +#define QEMU_QUIT_WAIT_SECONDS 5 static int qemuDomainDestroyFlags(virDomainPtr dom, @@ -1574,8 +1610,13 @@ qemuDomainDestroyFlags(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + bool use_thread = false; + bool use_kill = false; + virThread destroy_thread; + struct qemuDomainDestroyHelperData data; + unsigned long long then; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_MONITOR, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1590,12 +1631,14 @@ qemuDomainDestroyFlags(virDomainPtr dom, priv = vm->privateData; priv->fakeReboot = false; - /* Although qemuProcessStop does this already, there may - * be an outstanding job active. We want to make sure we - * can kill the process even if a job is active. Killing - * it now means the job will be released - */ - qemuProcessKill(vm); + if (!flags) { + /* Although qemuProcessStop does this already, there may + * be an outstanding job active. We want to make sure we + * can kill the process even if a job is active. Killing + * it now means the job will be released + */ + qemuProcessKill(vm); + } if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; @@ -1606,6 +1649,70 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto endjob; } + if (flags & VIR_DOMAIN_DESTROY_MONITOR) { + /* Try to shutdown domain gracefully. + * Send 'quit' to qemu. However, qemu can be unresponsive. + * Therefore create a separate thread in which we execute + * that monitor comand. Wait max QEMU_QUIT_WAIT_SECONDS. + */ + if (virMutexInit(&data.lock) < 0) { + virReportOOMError(); + goto endjob; + } + + if (virCondInit(&data.cond) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize thread condition")); + virMutexDestroy(&data.lock); + goto endjob; + } + + data.driver = driver; + data.vm = vm; + data.priv = priv; + data.guard = false; + + if (virThreadCreate(&destroy_thread, true, + qemuDomainDestroyHelper, &data) < 0) { + virReportSystemError(errno, "%s", + _("unable to create destroy thread")); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + goto endjob; + } + use_thread = true; + + if (virTimeMs(&then) < 0) + goto endjob; + + then += QEMU_QUIT_WAIT_SECONDS * 1000; + + /* How synchronization with destroy thread works: + * Basically wait on data.cond. But because conditions + * does not 'remember' that they have been signalized + * data.guard is used. Practically, data.guard says + * to destroy thread if we are waiting on condition + * and to us whether we should even try. + */ + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + if (virCondWaitUntil(&data.cond, &data.lock, then) < 0) { + if (errno == ETIMEDOUT) { + use_kill = true; + data.guard = false; + } else { + virMutexUnlock(&data.lock); + goto endjob; + } + } + } + virMutexUnlock(&data.lock); + + if (use_kill) + qemuProcessKill(vm); + } + qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_DESTROYED); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -1626,6 +1733,17 @@ endjob: vm = NULL; cleanup: + if (use_thread) { + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + ignore_value(virCondWait(&data.cond, &data.lock)); + } + virMutexUnlock(&data.lock); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + virThreadJoin(&destroy_thread); + } if (vm) virDomainObjUnlock(vm); if (event) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..41b9c5c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2475,3 +2475,16 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, ret = qemuMonitorTextBlockJob(mon, device, bandwidth, info, mode); return ret; } + +int qemuMonitorQuit(qemuMonitorPtr mon) +{ + int ret; + + VIR_DEBUG("mon=%p", mon); + + if (mon->json) + ret = qemuMonitorJSONQuit(mon); + else + ret = qemuMonitorTextQuit(mon); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..3fe6bb9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -475,6 +475,8 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorQuit(qemuMonitorPtr mon); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..1f078cd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2956,3 +2956,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONQuit(qemuMonitorPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("quit", NULL); + + if (!cmd) + return -1; + + ret =qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..2a7df76 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -231,4 +231,5 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorJSONQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 335e39e..19c2690 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3046,3 +3046,23 @@ cleanup: VIR_FREE(reply); return ret; } + + +int qemuMonitorTextQuit(qemuMonitorPtr mon) +{ + const char *cmd = "quit"; + char *reply = NULL; + int ret = -1; + + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..9ade938 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -224,4 +224,5 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorTextQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_TEXT_H */ diff --git a/tools/virsh.c b/tools/virsh.c index f1eb4ca..0c18d8b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2565,6 +2565,7 @@ static const vshCmdInfo info_destroy[] = { static const vshCmdOptDef opts_destroy[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"monitor", VSH_OT_BOOL, 0, N_("try graceful destroy via monitor first")}, {NULL, 0, 0, NULL} }; @@ -2574,6 +2575,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2581,7 +2583,11 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainDestroy(dom) == 0) { + if (vshCommandOptBool(cmd, "monitor")) + flags |= VIR_DOMAIN_DESTROY_MONITOR; + + if ((!flags && virDomainDestroy(dom) == 0) || + virDomainDestroyFlags(dom, flags) == 0) { vshPrint(ctl, _("Domain %s destroyed\n"), name); } else { vshError(ctl, _("Failed to destroy domain %s"), name); -- 1.7.3.4

5 14

[libvirt] [PATCH] esx: Use $(PYTHON) instead of the shebang to run the generator
by Matthias Bolte 24 Aug '11

24 Aug '11

--- src/Makefile.am | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8fe7120..5ba189c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -812,8 +812,9 @@ endif BUILT_SOURCES += $(ESX_DRIVER_GENERATED) -$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input $(srcdir)/esx/esx_vi_generator.py - $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py +$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input \ + $(srcdir)/esx/esx_vi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(PYTHON) $(srcdir)/esx/esx_vi_generator.py if WITH_ESX if WITH_DRIVER_MODULES -- 1.7.4.1

2 2

[libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
by Adam Litke 24 Aug '11

24 Aug '11

From: Osier Yang <jyang(a)redhat.com> * src/qemu/qemu_monitor_json.c: Handle error "CommandNotFound" and report the error. * src/qemu/qemu_monitor_text.c: If a sub info command is not found, it prints the output of "help info", for other commands, "unknown command" is printed. Without this patch, libvirt always report: An error occurred, but the cause is unknown This patch was adapted from a patch by Osier Yang <jyang(a)redhat.com> to break out detection of unrecognized text monitor commands into a separate function. Signed-off-by: Adam Litke <agl(a)us.ibm.com> --- src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++----------- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..715b26e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - - if (mode == BLOCK_JOB_ABORT) - cmd = qemuMonitorJSONMakeCommand("block_job_cancel", - "s:device", device, NULL); - else if (mode == BLOCK_JOB_INFO) - cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL); - else if (mode == BLOCK_JOB_SPEED) - cmd = qemuMonitorJSONMakeCommand("block_job_set_speed", - "s:device", device, - "U:value", bandwidth * 1024ULL * 1024ULL, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "query-block-jobs"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, "U:value", + bandwidth * 1024ULL * 1024ULL, NULL); - else if (mode == BLOCK_JOB_PULL) - cmd = qemuMonitorJSONMakeCommand("block_stream", - "s:device", device, NULL); + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, NULL); + } if (!cmd) return -1; @@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..9119f06 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -271,6 +271,20 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, scm_fd, reply); } +/* Check monitor output for evidence that the command was not recognized. + * For 'info' commands, qemu returns help text. For other commands, qemu + * returns 'unknown command:'. + */ +static int +qemuMonitorTextCommandNotFound(const char *reply) +{ + if (strstr(reply, "unknown command:")) + return 1; + if (strstr(reply, "info version")) + return 1; + return 0; +} + static int qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, @@ -3031,18 +3045,24 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, char *cmd = NULL; char *reply = NULL; int ret; - - if (mode == BLOCK_JOB_ABORT) - ret = virAsprintf(&cmd, "block_job_cancel %s", device); - else if (mode == BLOCK_JOB_INFO) - ret = virAsprintf(&cmd, "info block-jobs"); - else if (mode == BLOCK_JOB_SPEED) - ret = virAsprintf(&cmd, "block_job_set_speed %s %llu", device, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "info block-jobs"; + ret = virAsprintf(&cmd, "%s", cmd_name); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + ret = virAsprintf(&cmd, "%s %s %llu", cmd_name, device, bandwidth * 1024ULL * 1024ULL); - else if (mode == BLOCK_JOB_PULL) - ret = virAsprintf(&cmd, "block_stream %s", device); - else + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else { return -1; + } if (ret < 0) { virReportOOMError(); @@ -3056,6 +3076,13 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + if (qemuMonitorTextCommandNotFound(reply)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.3

2 3

[libvirt] [RFC 00/12] add support for new USB2 emulation / EHCI device
by Marc-André Lureau 24 Aug '11

24 Aug '11

Hi, The following patch intents to implement support for new USB2 and USB redirection added in QEMU 0.15. It follows somewhat Daniel's plan send earlier on this ML: https://www.redhat.com/archives/libvir-list/2011-August/msg00816.html There are some parts that I don't really know what they should be like: the "autoassign" or the default values at the domain XML level. Related RHBZ https://bugzilla.redhat.com/show_bug.cgi?id=725670 cheers Marc-André Lureau (12): Add various USB devices QEMU_CAPS Add USB controller models Add a new controller type 'usb' with optionnal 'model' USB controller can have a PCI address child element USB devices gain a new USB address child element Add USB companion controllers support Add USB hub device Modify USB port to be defined as a port path RFC: Don't reserve slot 1 if a USB controller is defined there RFC: Don't append 0 at usb id, so that it is compatible with legacy -usb Add a usb1 & usb2 qemuxml2argv test Add usb-redir device docs/formatdomain.html.in | 79 +++++- docs/schemas/domain.rng | 124 ++++++-- src/conf/domain_conf.c | 333 +++++++++++++++++++- src/conf/domain_conf.h | 59 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 34 ++ src/qemu/qemu_capabilities.h | 11 + src/qemu/qemu_command.c | 262 ++++++++++++++-- src/qemu/qemu_command.h | 10 +- src/qemu/qemu_hotplug.c | 12 +- src/vmx/vmx.c | 12 +- tests/qemuhelptest.c | 16 +- .../qemuxml2argv-input-usbmouse-addr.args | 1 + .../qemuxml2argv-input-usbmouse-addr.xml | 27 ++ .../qemuxml2argv-usb-controller.args | 1 + .../qemuxml2argv-usb-controller.xml | 16 + tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml | 19 ++ .../qemuxml2argv-usb-ich9-companion.args | 6 + .../qemuxml2argv-usb-ich9-companion.xml | 30 ++ .../qemuxml2argv-usb-ich9-ehci-addr.args | 1 + .../qemuxml2argv-usb-ich9-ehci-addr.xml | 18 + .../qemuxml2argv-usb-piix3-controller.args | 1 + .../qemuxml2argv-usb-piix3-controller.xml | 16 + tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml | 31 ++ tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 8 + tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 33 ++ tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 74 +++++ tests/qemuxml2argvtest.c | 34 ++ 31 files changed, 1196 insertions(+), 77 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml -- 1.7.6

4 21

[libvirt] [PATCH] esx: Refactor a repeated string in the generator
by Matthias Bolte 23 Aug '11

23 Aug '11

--- src/esx/esx_vi_generator.py | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 239ec73..8a128df 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1817,17 +1817,19 @@ for obj in managed_objects_by_name.values(): -types_typedef.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -types_typeenum.write("/* Generated by esx_vi_generator.py */\n\n") -types_typetostring.write("/* Generated by esx_vi_generator.py */\n\n") -types_typefromstring.write("/* Generated by esx_vi_generator.py */\n\n") -types_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -types_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -methods_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -methods_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -methods_macro.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -helpers_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -helpers_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") +notice = "/* Generated by esx_vi_generator.py */\n\n\n\n" + +types_typedef.write(notice) +types_typeenum.write(notice) +types_typetostring.write(notice) +types_typefromstring.write(notice) +types_header.write(notice) +types_source.write(notice) +methods_header.write(notice) +methods_source.write(notice) +methods_macro.write(notice) +helpers_header.write(notice) +helpers_source.write(notice) -- 1.7.4.1

2 2

[libvirt] Curl/HTTP block device
by Radek Hladik 23 Aug '11

23 Aug '11

Hi, I've noticed newer version of curl in last qemu changelog. It seems that its needed for curl block device. Unfortunatelly, I was not able to find out more. I tried to look into qemu source code and to some patches but I am not much wiser. As I understand it, qemu could be able to use a curl compatible URL as block device. Probably only in readonly mode but it still would be very usefull for i.e. ISO images. Can anyone point me in the right direction and more importantly, is this feature available via Libvirt? I checked the documentation on libvirt.org and it seems that remode disk can be only nbd/rbd/sheepdog. Radek

3 3

[libvirt] [PATCH] maint: fix comment typos
by Eric Blake 23 Aug '11

23 Aug '11

* src/qemu/qemu_driver.c (qemuDomainSaveInternal): Fix typo. * src/conf/domain_event.c (virDomainEventDispatchMatchCallback): Likewise. * daemon/libvirtd.c (daemonRunStateInit): Likewise. * src/lxc/lxc_container.c (lxcContainerChildMountSort): Likewise. * src/util/virterror.c (virCopyError, virRaiseErrorFull): Likewise. * src/xenxs/xen_sxpr.c (xenParseSxprSound): Likewise. --- Pushing under the trivial rule. daemon/libvirtd.c | 2 +- src/conf/domain_event.c | 4 ++-- src/lxc/lxc_container.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/util/virterror.c | 4 ++-- src/xenxs/xen_sxpr.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 0530ba5..5969a82 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1131,7 +1131,7 @@ static void daemonRunStateInit(void *opaque) virNetServerPtr srv = opaque; /* Start the stateful HV drivers - * This is delibrately done after telling the parent process + * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will * seriously delay OS bootup process */ if (virStateInitialize(virNetServerIsPrivileged(srv)) < 0) { diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 2e0524b..3189346 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1103,11 +1103,11 @@ static int virDomainEventDispatchMatchCallback(virDomainEventPtr event, return 0; if (cb->dom) { - /* Delibrately ignoring 'id' for matching, since that + /* Deliberately ignoring 'id' for matching, since that * will cause problems when a domain switches between * running & shutoff states & ignoring 'name' since * Xen sometimes renames guests during migration, thus - * leaving 'uuid' as the only truely reliable ID we can use*/ + * leaving 'uuid' as the only truly reliable ID we can use*/ if (memcmp(event->dom.uuid, cb->dom->uuid, VIR_UUID_BUFLEN) == 0) return 1; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 895ef09..e425328 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -287,7 +287,7 @@ static int lxcContainerChildMountSort(const void *a, const void *b) const char **sa = (const char**)a; const char **sb = (const char**)b; - /* Delibrately reversed args - we need to unmount deepest + /* Deliberately reversed args - we need to unmount deepest children first */ return strcmp(*sb, *sa); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 035ddb4..2c677b0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2564,7 +2564,7 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom, /* Avoid throwing an error here, since it is possible * that with NFS we can't actually stat() the file. * The subsequent codepaths will still raise an error - * if a truely fatal problem is hit */ + * if a truly fatal problem is hit */ is_reg = true; } else { is_reg = !!S_ISREG(sb.st_mode); diff --git a/src/util/virterror.c b/src/util/virterror.c index 9a27feb..c5babb1 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -248,7 +248,7 @@ virCopyError(virErrorPtr from, to->int1 = from->int1; to->int2 = from->int2; /* - * Delibrately not setting 'conn', 'dom', 'net' references + * Deliberately not setting 'conn', 'dom', 'net' references */ return ret; } @@ -708,7 +708,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, * Save the information about the error */ /* - * Delibrately not setting conn, dom & net fields since + * Deliberately not setting conn, dom & net fields since * they're utterly unsafe */ to->domain = domain; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 7b292bc..5e278ae 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -604,7 +604,7 @@ xenParseSxprSound(virDomainDefPtr def, * Special compatability code for Xen with a bogus * sound=all in config. * - * NB delibrately, don't include all possible + * NB deliberately, don't include all possible * sound models anymore, just the 2 that were * historically present in Xen's QEMU. * -- 1.7.4.4

1 0

[libvirt] [PATCH v4] Add support for fd: protocol
by Corey Bryant 23 Aug '11

23 Aug '11

sVirt provides SELinux MAC isolation for Qemu guest processes and their corresponding resources (image files). sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, which is needed for image file isolation when using the sVirt SELinux security driver in libvirt. The proposed solution entails a combination of Qemu, libvirt, and SELinux patches that work together to isolate multiple guests' images when they're stored in the same NFS mount. This results in an environment where sVirt isolation and NFS image file isolation can both be provided. Currently, Qemu opens an image file in addition to performing the necessary read and write operations. The proposed solution will move the open out of Qemu and into libvirt. Once libvirt opens an image file for the guest, it will pass the file descriptor to Qemu via a new fd: protocol. If the image file resides in an NFS mount, the following SELinux policy changes will provide image isolation: - A new SELinux boolean is created (e.g. virt_read_write_nfs) to allow Qemu (svirt_t) to only have SELinux read and write permissions on nfs_t files - Qemu (svirt_t) also gets SELinux use permissions on libvirt (virtd_t) file descriptors Following is a sample invocation of Qemu using the fd: protocol on the command line: qemu -drive file=fd:4,format=qcow2 The fd: protocol is also supported by the drive_add monitor command. This requires that the specified file descriptor is passed to the monitor alongside a prior getfd monitor command. This patch also supports the following features for the fd: protocol: - -snapshot command line option - savevm monitor command This patch does not contain support for the following features, all of which are planned to be supported in the future: - Copy-on-write backing files - snapshot_blkdev monitor command - -cdrom command line option - -drive command line option with media=cdrom - change monitor command v2: - Add drive_add monitor command support - Fence off unsupported features that re-open image file v3: - Fence off cdrom and change monitor command support v4: - Removed checks that fenced off features for fd: protocol - Enabled -snapshot and savevm support Signed-off-by: Corey Bryant <coreyb(a)linux.vnet.ibm.com> --- block.c | 9 ++--- block/raw-posix.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++---- migration-fd.c | 2 +- monitor.c | 39 ++++++++++++-------- monitor.h | 3 +- net.c | 2 +- qemu-options.hx | 8 +++-- qemu-tool.c | 5 +++ 8 files changed, 131 insertions(+), 36 deletions(-) diff --git a/block.c b/block.c index 785c88e..e9e5613 100644 --- a/block.c +++ b/block.c @@ -555,7 +555,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, if (flags & BDRV_O_SNAPSHOT) { BlockDriverState *bs1; int64_t total_size; - int is_protocol = 0; BlockDriver *bdrv_qcow2; QEMUOptionParameter *options; char tmp_filename[PATH_MAX]; @@ -573,19 +572,17 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, } total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK; - if (bs1->drv && bs1->drv->protocol_name) - is_protocol = 1; - bdrv_delete(bs1); get_tmp_filename(tmp_filename, sizeof(tmp_filename)); /* Real path is meaningless for protocols */ - if (is_protocol) + if (path_has_protocol(filename)) { snprintf(backing_filename, sizeof(backing_filename), "%s", filename); - else if (!realpath(filename, backing_filename)) + } else if (!realpath(filename, backing_filename)) { return -errno; + } bdrv_qcow2 = bdrv_find_format("qcow2"); options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); diff --git a/block/raw-posix.c b/block/raw-posix.c index c5c9944..42ae2f4 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -28,6 +28,7 @@ #include "block_int.h" #include "module.h" #include "block/raw-posix-aio.h" +#include "monitor.h" #ifdef CONFIG_COCOA #include <paths.h> @@ -185,7 +186,8 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, int bdrv_flags, int open_flags) { BDRVRawState *s = bs->opaque; - int fd, ret; + int fd = -1; + int ret; ret = raw_normalize_devicepath(&filename); if (ret != 0) { @@ -207,13 +209,21 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, if (!(bdrv_flags & BDRV_O_CACHE_WB)) s->open_flags |= O_DSYNC; - s->fd = -1; - fd = qemu_open(filename, s->open_flags, 0644); - if (fd < 0) { - ret = -errno; - if (ret == -EROFS) - ret = -EACCES; - return ret; + if (s->fd == -1) { + fd = qemu_open(filename, s->open_flags, 0644); + if (fd < 0) { + ret = -errno; + if (ret == -EROFS) { + ret = -EACCES; + } + return ret; + } + } else { + fd = dup(s->fd); + if (fd < 0) { + ret = -errno; + return ret; + } } s->fd = fd; s->aligned_buf = NULL; @@ -271,6 +281,7 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; + s->fd = -1; s->type = FTYPE_FILE; return raw_open_common(bs, filename, flags, 0); } @@ -904,6 +915,74 @@ static BlockDriver bdrv_file = { .create_options = raw_create_options, }; +static int raw_open_fd(BlockDriverState *bs, const char *filename, int flags) +{ + BDRVRawState *s = bs->opaque; + const char *fd_str; + int fd; + + /* extract the file descriptor - fail if it's not fd: */ + if (!strstart(filename, "fd:", &fd_str)) { + return -EINVAL; + } + + if (!qemu_isdigit(fd_str[0])) { + /* get fd from monitor, but don't remove from monitor's fd list */ + fd = qemu_get_fd(fd_str, 0); + if (fd == -1) { + return -EBADF; + } + } else { + char *endptr = NULL; + + fd = strtol(fd_str, &endptr, 10); + if (*endptr || (fd == 0 && fd_str == endptr)) { + return -EBADF; + } + } + + s->fd = fd; + s->type = FTYPE_FILE; + + return raw_open_common(bs, filename, flags, 0); +} + +static void raw_close_fd(BlockDriverState *bs) +{ + const char *fd_str; + + /* if file descriptor is an fdname, remove it from monitor's fd list */ + if (strstart(bs->filename, "fd:", &fd_str)) { + if (!qemu_isdigit(fd_str[0])) { + qemu_get_fd(fd_str, 1); + } + } + + raw_close(bs); +} + +static BlockDriver bdrv_file_fd = { + .format_name = "file", + .protocol_name = "fd", + .instance_size = sizeof(BDRVRawState), + .bdrv_probe = NULL, /* no probe for protocols */ + .bdrv_file_open = raw_open_fd, + .bdrv_read = raw_read, + .bdrv_write = raw_write, + .bdrv_close = raw_close_fd, + .bdrv_flush = raw_flush, + .bdrv_discard = raw_discard, + + .bdrv_aio_readv = raw_aio_readv, + .bdrv_aio_writev = raw_aio_writev, + .bdrv_aio_flush = raw_aio_flush, + + .bdrv_truncate = raw_truncate, + .bdrv_getlength = raw_getlength, + + .create_options = raw_create_options, +}; + /***********************************************/ /* host device */ @@ -1012,6 +1091,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) } #endif + s->fd = -1; s->type = FTYPE_FILE; #if defined(__linux__) { @@ -1184,6 +1264,7 @@ static int floppy_open(BlockDriverState *bs, const char *filename, int flags) BDRVRawState *s = bs->opaque; int ret; + s->fd = -1; s->type = FTYPE_FD; /* open will not fail even if no floppy is inserted, so add O_NONBLOCK */ @@ -1302,6 +1383,7 @@ static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; + s->fd = -1; s->type = FTYPE_CD; /* open will not fail even if no CD is inserted, so add O_NONBLOCK */ @@ -1527,6 +1609,7 @@ static void bdrv_file_init(void) * Register all the drivers. Note that order is important, the driver * registered last will get probed first. */ + bdrv_register(&bdrv_file_fd); bdrv_register(&bdrv_file); bdrv_register(&bdrv_host_device); #ifdef __linux__ diff --git a/migration-fd.c b/migration-fd.c index aee690a..b1ba625 100644 --- a/migration-fd.c +++ b/migration-fd.c @@ -61,7 +61,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon, s = g_malloc0(sizeof(*s)); - s->fd = monitor_get_fd(mon, fdname); + s->fd = monitor_get_fd(mon, fdname, 1); if (s->fd == -1) { DPRINTF("fd_migration: invalid file descriptor identifier\n"); goto err_after_alloc; diff --git a/monitor.c b/monitor.c index 39791dc..d03e762 100644 --- a/monitor.c +++ b/monitor.c @@ -1198,21 +1198,21 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d qerror_report(QERR_DEVICE_NOT_ACTIVE, "spice"); return -1; } - qerror_report(QERR_ADD_CLIENT_FAILED); - return -1; + qerror_report(QERR_ADD_CLIENT_FAILED); + return -1; #ifdef CONFIG_VNC } else if (strcmp(protocol, "vnc") == 0) { - int fd = monitor_get_fd(mon, fdname); - vnc_display_add_client(NULL, fd, skipauth); - return 0; + int fd = monitor_get_fd(mon, fdname, 1); + vnc_display_add_client(NULL, fd, skipauth); + return 0; #endif } else if ((s = qemu_chr_find(protocol)) != NULL) { - int fd = monitor_get_fd(mon, fdname); - if (qemu_chr_add_client(s, fd) < 0) { - qerror_report(QERR_ADD_CLIENT_FAILED); - return -1; - } - return 0; + int fd = monitor_get_fd(mon, fdname, 1); + if (qemu_chr_add_client(s, fd) < 0) { + qerror_report(QERR_ADD_CLIENT_FAILED); + return -1; + } + return 0; } qerror_report(QERR_INVALID_PARAMETER, "protocol"); @@ -2833,7 +2833,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) } } -int monitor_get_fd(Monitor *mon, const char *fdname) +int monitor_get_fd(Monitor *mon, const char *fdname, unsigned char remove) { mon_fd_t *monfd; @@ -2846,10 +2846,12 @@ int monitor_get_fd(Monitor *mon, const char *fdname) fd = monfd->fd; - /* caller takes ownership of fd */ - QLIST_REMOVE(monfd, next); - g_free(monfd->name); - g_free(monfd); + if (remove) { + /* caller takes ownership of fd */ + QLIST_REMOVE(monfd, next); + g_free(monfd->name); + g_free(monfd); + } return fd; } @@ -2857,6 +2859,11 @@ int monitor_get_fd(Monitor *mon, const char *fdname) return -1; } +int qemu_get_fd(const char *fdname, unsigned char remove) +{ + return cur_mon ? monitor_get_fd(cur_mon, fdname, remove) : -1; +} + static const mon_cmd_t mon_cmds[] = { #include "hmp-commands.h" { NULL, NULL, }, diff --git a/monitor.h b/monitor.h index 4f2d328..f18de19 100644 --- a/monitor.h +++ b/monitor.h @@ -50,7 +50,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs, BlockDriverCompletionFunc *completion_cb, void *opaque); -int monitor_get_fd(Monitor *mon, const char *fdname); +int monitor_get_fd(Monitor *mon, const char *fdname, unsigned char remove); +int qemu_get_fd(const char *fdname, unsigned char remove); void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); diff --git a/net.c b/net.c index d05930c..3482598 100644 --- a/net.c +++ b/net.c @@ -727,7 +727,7 @@ int net_handle_fd_param(Monitor *mon, const char *param) if (!qemu_isdigit(param[0]) && mon) { - fd = monitor_get_fd(mon, param); + fd = monitor_get_fd(mon, param, 1); if (fd == -1) { error_report("No file descriptor named %s found", param); return -1; diff --git a/qemu-options.hx b/qemu-options.hx index d86815d..869320b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -131,7 +131,7 @@ using @file{/dev/cdrom} as filename (@pxref{host_drives}). ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, - "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" + "-drive [file=[fd:]file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" " [,cache=writethrough|writeback|none|unsafe][,format=f]\n" " [,serial=s][,addr=A][,id=name][,aio=threads|native]\n" @@ -144,10 +144,12 @@ STEXI Define a new drive. Valid options are: @table @option -@item file=@var{file} +@item file=[fd:]@var{file} This option defines which disk image (@pxref{disk_images}) to use with this drive. If the filename contains comma, you must double it -(for instance, "file=my,,file" to use file "my,file"). +(for instance, "file=my,,file" to use file "my,file"). @option{fd:}@var{file} +specifies the file descriptor of an already open disk image. +@option{format=}@var{format} is required by @option{fd:}@var{file}. @item if=@var{interface} This option defines on which type on interface the drive is connected. Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio. diff --git a/qemu-tool.c b/qemu-tool.c index eb89fe0..a2a0129 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -96,3 +96,8 @@ int64_t qemu_get_clock_ns(QEMUClock *clock) { return 0; } + +int qemu_get_fd(const char *fdname, unsigned char remove) +{ + return -1; +} -- 1.7.3.4

7 19

[libvirt] [PATCH] virsh: Return false if connection or domain is not available
by Osier Yang 23 Aug '11

23 Aug '11

Instead of goto cleanup lable, it will work fine if go to cleanup lable, but it's somewhat waste. And some functions don't check if the "domain == NULL" before trying to free it with virDomainFree(dom), as a result, there is additional error in the log says: error: invalid domain pointer in virDomainFree Which is useless. --- tools/virsh.c | 60 ++++++++++++++++++++++++++++---------------------------- 1 files changed, 30 insertions(+), 30 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7d849ec..ffb4ced 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5201,10 +5201,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, int ret = -1; if (!vshConnectionUsability(ctl, ctl->conn)) - goto out; + return -1; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) - goto out; + return -1; if (vshCommandOptString(cmd, "path", &path) < 0) goto out; @@ -10519,10 +10519,10 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) char *xml; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; @@ -10633,10 +10633,10 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; @@ -10922,10 +10922,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) char *xml; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "source", &source) <= 0) goto cleanup; @@ -11105,10 +11105,10 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "target", &target) <= 0) goto cleanup; @@ -11655,11 +11655,11 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain (ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; /* Get the XML configuration of the domain. */ doc = virDomainGetXMLDesc (dom, flags); @@ -11806,11 +11806,11 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) char *name = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "xmlfile", &from) <= 0) buffer = vshStrdup(ctl, "<domainsnapshot/>"); @@ -11902,11 +11902,11 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "name", &name) < 0 || vshCommandOptString(cmd, "description", &desc) < 0) { @@ -11995,11 +11995,11 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) char *xml = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; current = virDomainHasCurrentSnapshot(dom, 0); if (current < 0) @@ -12079,11 +12079,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) struct tm time_info; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; numsnaps = virDomainSnapshotNum(dom, 0); @@ -12186,11 +12186,11 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd) char *xml = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12245,11 +12245,11 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) xmlXPathContextPtr ctxt = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12311,11 +12311,11 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) virDomainSnapshotPtr snapshot = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12364,11 +12364,11 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12427,11 +12427,11 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) bool pad = false; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; while ((opt = vshCommandOptArgv(cmd, opt))) { if (pad) -- 1.7.6

2 5

[libvirt] ignore vs. error when inappropriate or unrecognized attributes/elements are present in XML
by Laine Stump 23 Aug '11

23 Aug '11

This is related to: https://bugzilla.redhat.com/show_bug.cgi?id=638633#c14 I had started to reply to it in the comments of the bug, but my reply became too long, and expanded into an issue wider than that single bug, so I figured I'd better discuss it here instead. A synopsis: It's possible to specify a script to be executed when bringing up a guest interface device. This is done by adding a <script path='xxx'/> element to the <interface> definition. The bug report was that <script path='blah'/> was added to an <interface type='bridge'> definition in the config for a qemu domain, and nobody complained that 'blah' didn't exist. This eventually led to the realization that when interface type='bridge' and driver=qemu, <script> isn't used, so it's ignored. More exactly, the script element is only recognized by the parser when interface type == 'bridge' or 'ethernet', and is only used by the hypervisor driver when: 1) the interface type='bridge' and the xen driver is being used. or 2) the interface type='ethernet' and the qemu driver is being used. After this all came to light, in comment 14, Dave says: > That's being the case, we need to explicitly reject the attempt to specify a > script. (implied: when interface type='bridge' and the hypervisor is qemu). On the surface that sounds like a good idea, but it's opening a pretty big can of worms. Specifically for this issue, this rejection must take place in the hypervisor because different hypervisors support scripts for different types of interfaces - a script for type='bridge' is perfectly acceptable for xen, but not for qemu. That can be handled with an error log in the qemu driver in the case of type='bridge'. But if someone defines <interface type='network'> and specifies a script, the parser ignores the <script> element (because normally it's stored either in a union that's only valid when type='bridge' or a different union only valid when type='ethernet'), so qemu has no way of knowing a script was specified and therefore can't log an error. That's not a terribly difficult problem, though. Just a bit more code than a simple "else { log an error; }" somewhere in the code - the parser needs to be changed to move the script attribute out of the unions and into the main struct, and always populate it, then the drivers need to be taught to log an error when appropriate). But if we're doing that for the <script> subelement of <interface>, we're creating a prerequisite for what's expected in the case of all the *other* attributes/elements that are currently ignored under similar circumstances by libvirt's parsers. Essentially anything that's in a union contained in any of the virXxxDef structs is probably treated the same way. As a simple first example, look at "keymap", which is an attribute of a domain's devices/graphics element, but only valid if the type is spice or vnc. The parser ignores it in all other cases, and since it's stored in data.spice or data.vnc, there's not even a possibility of qemu (or other hypervisor) indicating any kind of error when someone specifies e.g. <graphics type='sdl' 'keymap='blah' .../>. Similarly, if someone were to add "tlsPort='5910' to a <graphics> of any type other than spice. Of course, from the point of view of all the code associated with <graphics type='vnc' ... />, this is no different than if someone had added "frozzle='fib'" - it's totally unexpected (in this case by any type), so it's ignored. Actually, I can see now there are several different classes of this problem. Here are the first few that come to mind: 1) an attribute/element is completely unknown/unexpected in all cases (e.g. "frozzle='fib'" anywhere, or more insidious, something that *looks* correct, but isn't, e.g. "<script name='/path/to/script'/>"*) 2) an attribute/element is useful/expected only when some other attribute is set to a particular value (usually one called "type", but could be something else), for example keymap='blah' is only expected in a <graphics> element when type='spice' or type='vnc'. 3) an attribute/element is useful/expected only for certain combinations of the value of some other attribute and which driver is using the element, e.g. the subject of this bug - script='blah' is only expected when type='bridge' and it's used by the Xen driver, or type='ethernet' and it's used by the qemu driver. So what are the rules of engagement for these various cases? When do we ignore, when do we log an error during parsing, and when do we log an error in the code that's using the parsed data? (*Another example of (1) from recent real-life - during testing of https://bugzilla.redhat.com/show_bug.cgi?id=727982, a QA person claimed that the bugt hadn't been fixed because the hosts file wasn't being updated; it turned out that they were adding the new <host> element at the toplevel under <network> rather than inside the <dhcp> subelement, so the parser was ignoring it (since it's unrecognized at that level) rather than logging an error).

3 7

[libvirt] Draft document about source control for type of pools
by Lei Li 23 Aug '11

23 Aug '11

Hi Daniel, The last version of my patch fixed bug #611823 prohibit pools with duplicate storage, you gave some comments that should do check by source info instead of target path I used. These days I view the code and existing documents about storage pool, and tried to code based on your comments, but looks like it is turning out to be more complex. First, I think it is possible to make clear what it means for a pool to be a duplicate. Below is the detailed source info be used for each type of pool according to the existing documents and the main item to control. For details of each type of pool: VIR_STORAGE_POOL_DIR, /* For dir type, use source->dir only(But in the code and example XML file,it use target path).*/ A pool with a type of dir provides the means to manage files within a directory. VIR_STORAGE_POOL_FS, /* For fs type, use source->device->path.*/ This is a variant of the directory pool. Instead of creating a directory on an existing mounted filesystem though, it expects a source block device to be named. This block device will be mounted and files managed in the directory of its mount point. VIR_STORAGE_POOL_NETFS, /* For netfs, use host name and source->dir */ This is a variant of the filesystem pool. Instead of requiring a local block device as the source, it requires the name of a host and path of an exported directory. It will mount this network filesystem and manage files within the directory of its mount point. VIR_STORAGE_POOL_LOGICAL, /* For logical, use source->device->path. */ This provides a pool based on an LVM volume group. For a pre-defined LVM volume group, simply providing the group name is sufficient,while to build a new group requires providing a list of source devices to serve as physical volumes. VIR_STORAGE_POOL_DISK, /* For disk type, use source->device->path.*/ This provides a pool based on a physical disk. Volumes are created by adding partitions to the disk. VIR_STORAGE_POOL_ISCSI, /* For ISCSI type, use source->device->path. */ This provides a pool based on an iSCSI target. VIR_STORAGE_POOL_SCSI, /* For SCSI type, use source->adapter name. */ This provides a pool based on a SCSI HBA. VIR_STORAGE_POOL_MPATH, This provides a pool that contains all the multipath devices on the host. Volume creating is not supported via the libvirt APIs, so we will just ignore this type. After a preliminary investigation, for source element, source->device->path, -Pool type: fs, logical, disk, ISCSI. -May only occur once. source->dir, -Pool type: dir, netfs. -May only occur once. source->adapter -Pool type: SCSI. -May only occur once. are the main required location info, and we can control the duplicate storage based on these three item above for related type of pool. Second, I have tried to do check by source info for a test, when I debug, I found that pools->objs[]->def->source.* which come from virConnectPtr where all existing pools info be keeped == NULL, but the current target pool virStoragePoolDefPtr def can get source field value, so I think libvirt didn't get source field info in the pools list where we do check at all today. Do you think we should do check based on this? Or any comments for question one and two. After you confirmed then I will work on code. Thank you. -- Lei

2 1

[libvirt] hi, Is libvirt plan to support open vswitch in the future? thanks
by c02925 23 Aug '11

23 Aug '11

Hello, Is libvirt plan to support open vswitch in the future, or there is already has a blueprint about it. Has anyone else thought about this problem? Any related information is welcome. Many thanks, zhifeng cai. <http://www.linkedin.com/in/aliyev>

1 0

[libvirt] [PATCH] qemu: Substitute VIR_ERR_NO_SUPPORT with VIR_ERR_OPERATION_INVALID
by Osier Yang 23 Aug '11

23 Aug '11

* src/qemu/qemu_monitor_text.c: Error like "this function is not supported by the connection driver" is confused obviously. --- src/qemu/qemu_monitor_text.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index d296675..4a7c242 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -713,7 +713,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, * to detect if qemu supports the command. */ if (strstr(info, "\ninfo ")) { - qemuReportError(VIR_ERR_NO_SUPPORT, + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("'info blockstats' not supported by this qemu")); goto cleanup; @@ -1388,7 +1388,7 @@ int qemuMonitorTextMigrate(qemuMonitorPtr mon, /* If the command isn't supported then qemu prints: * unknown command: migrate" */ if (strstr(info, "unknown command:")) { - qemuReportError(VIR_ERR_NO_SUPPORT, + qemuReportError(VIR_ERR_OPERATION_INVALID, _("migration to '%s' not supported by this qemu: %s"), dest, info); goto cleanup; } @@ -1652,7 +1652,7 @@ int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon, } if (strstr(reply, "invalid type: host")) { - qemuReportError(VIR_ERR_NO_SUPPORT, "%s", + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("PCI device assignment is not supported by this version of qemu")); goto cleanup; } -- 1.7.6

2 4

[libvirt] [PATCH] virsh: Print error if specified bandwidth is invalid for blockjob
by Osier Yang 23 Aug '11

23 Aug '11

It's strange that the command fails but without any error if one specifies as not a number. --- tools/virsh.c | 21 ++++++++++++++++----- 1 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ffb4ced..97512b5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5209,17 +5209,28 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(cmd, "path", &path) < 0) goto out; - if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) + if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); goto out; + } - if (mode == VSH_CMD_BLOCK_JOB_ABORT) + if (mode == VSH_CMD_BLOCK_JOB_ABORT) { + vshPrint(ctl, "abort"); ret = virDomainBlockJobAbort(dom, path, 0); - else if (mode == VSH_CMD_BLOCK_JOB_INFO) + } + else if (mode == VSH_CMD_BLOCK_JOB_INFO) { + vshPrint(ctl, "info\n"); ret = virDomainGetBlockJobInfo(dom, path, info, 0); - else if (mode == VSH_CMD_BLOCK_JOB_SPEED) + vshPrint(ctl, "ret = %d\n", ret); + } + else if (mode == VSH_CMD_BLOCK_JOB_SPEED) { + vshPrint(ctl, "speed"); ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0); - else if (mode == VSH_CMD_BLOCK_JOB_PULL) + } + else if (mode == VSH_CMD_BLOCK_JOB_PULL) { + vshPrint(ctl, "pull"); ret = virDomainBlockPull(dom, path, bandwidth, 0); + } out: virDomainFree(dom); -- 1.7.6

2 3

[libvirt] Adding migration speed to domainXML
by Jim Fehlig 22 Aug '11

22 Aug '11

I finally have some time to investigate Eric's suggestion [1] to add virDomainMigrateGetMaxSpeed API and track current value in domainXML. WRT the XML representation, it seems this best fits under the existing lifecycle controls, e.g. a new "on_migrate" element. I'm not sure if this should be kept simple and rather flat such as <on_migrate max-speed='1000000000' /> or a little more expressive and extensible with something like <on_migrate> <bandwidth peak='1000000000' /> </on_migrate> Any comments on these representations or perhaps alternative suggestions? Regards, Jim [1] https://www.redhat.com/archives/libvir-list/2011-August/msg00224.html

1 0

[libvirt] Fwd: [virt-tools-list] virt-manager, dependency question
by Jason Helfman 22 Aug '11

22 Aug '11

Can anyone please confirm this for me? Thanks! Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5

3 2

[libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
by Osier Yang 22 Aug '11

22 Aug '11

* src/qemu/qemu_monitor_json.c: Handle error "CommandNotFound" and report the error. * src/qemu/qemu_monitor_text.c: If a sub info command is not found, it prints the output of "help info", for other commands, "unknown command" is printed. Without this patch, libvirt always report: An error occurred, but the cause is unknown --- src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++----------- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..715b26e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - - if (mode == BLOCK_JOB_ABORT) - cmd = qemuMonitorJSONMakeCommand("block_job_cancel", - "s:device", device, NULL); - else if (mode == BLOCK_JOB_INFO) - cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL); - else if (mode == BLOCK_JOB_SPEED) - cmd = qemuMonitorJSONMakeCommand("block_job_set_speed", - "s:device", device, - "U:value", bandwidth * 1024ULL * 1024ULL, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "query-block-jobs"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, "U:value", + bandwidth * 1024ULL * 1024ULL, NULL); - else if (mode == BLOCK_JOB_PULL) - cmd = qemuMonitorJSONMakeCommand("block_stream", - "s:device", device, NULL); + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, NULL); + } if (!cmd) return -1; @@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..d296675 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3031,18 +3031,24 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, char *cmd = NULL; char *reply = NULL; int ret; - - if (mode == BLOCK_JOB_ABORT) - ret = virAsprintf(&cmd, "block_job_cancel %s", device); - else if (mode == BLOCK_JOB_INFO) - ret = virAsprintf(&cmd, "info block-jobs"); - else if (mode == BLOCK_JOB_SPEED) - ret = virAsprintf(&cmd, "block_job_set_speed %s %llu", device, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "info block-jobs"; + ret = virAsprintf(&cmd, "%s", cmd_name); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + ret = virAsprintf(&cmd, "%s %s %llu", cmd_name, device, bandwidth * 1024ULL * 1024ULL); - else if (mode == BLOCK_JOB_PULL) - ret = virAsprintf(&cmd, "block_stream %s", device); - else + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else { return -1; + } if (ret < 0) { virReportOOMError(); @@ -3056,6 +3062,27 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + /* Check error: Command not found, for "info block-jobs" command. + * If qemu monitor command "info" doesn't support one sub-command, + * it will print the output of "help info" instead, so simply check + * if "info version" is in the output. + */ + if (mode == BLOCK_JOB_INFO) { + if (strstr(reply, "info version")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + } else { + if (strstr(reply, "unknown command")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.6

3 3

[libvirt] [PATCH] blockjob: Report correct error when missing qemu support
by Adam Litke 22 Aug '11

22 Aug '11

When executing blockjob monitor commands, we are not checking for CommandNotFound errors. This results in 'unexpected error' messages when using a qemu without support for the required commands. Fix this up by checking for CommandNotFound errors for QMP and detecting this condition for the text monitor. This fixes the problem reported in Bug 727502. Signed-off-by: Adam Litke <agl(a)us.ibm.com> --- src/qemu/qemu_monitor_json.c | 3 +++ src/qemu/qemu_monitor_text.c | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..4c8a08b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2939,6 +2939,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Operation is not supported")); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..d9cad2d 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -271,6 +271,19 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, scm_fd, reply); } +/* Check monitor output for evidence that the command was not recognized. + * For 'info' commands, qemu returns help text. For other commands, qemu + * returns 'unknown command:'. + */ +static int +qemuMonitorTextCommandNotFound(const char *reply) +{ + if (strstr(reply, "unknown command:")) + return 1; + if (strstr(reply, "info version -- show the version of QEMU")) + return 1; + return 0; +} static int qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, @@ -3056,6 +3069,13 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + if (qemuMonitorTextCommandNotFound(reply)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Operation is not supported")); + ret = -1; + goto cleanup; + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.3

2 1

[libvirt] [RFC 0/4] Report disk latency information
by Osier Yang 22 Aug '11

22 Aug '11

The QEMU side patches are not posted yet, but considering what these patche series need is only a fixed interface provided by QEMU, posting it for early reviewing, won't push it before QEMU patches are got pushed even it got ACK. Osier

6 13

[libvirt] [PATCH] remote: Switch to TLS when connecting to remote socket
by Michal Privoznik 22 Aug '11

22 Aug '11

If users wants to connect to remote unix socket, e.g. 'qemu+unix://<remote>/system' currently the <remote> part is ignored, ending up connecting to localhost. This patch solves this issue by creating TLS connection. --- src/remote/remote_driver.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e5bfa4b..630c8ea 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -325,9 +325,14 @@ doRemoteOpen (virConnectPtr conn, } else { if (STRCASEEQ (transport_str, "tls")) transport = trans_tls; - else if (STRCASEEQ (transport_str, "unix")) - transport = trans_unix; - else if (STRCASEEQ (transport_str, "ssh")) + else if (STRCASEEQ (transport_str, "unix")) { + /* When connecting to unix socket on remote + * machine use TLS to connect to it */ + if (conn->uri->server) + transport = trans_tls; + else + transport = trans_unix; + } else if (STRCASEEQ (transport_str, "ssh")) transport = trans_ssh; else if (STRCASEEQ (transport_str, "ext")) transport = trans_ext; -- 1.7.3.4

2 1

[libvirt] [PATCH] [BUGFIX][PATCH] send-key: fix scan keycode map
by KAMEZAWA Hiroyuki 22 Aug '11

22 Aug '11

Now, bad key-code in send-key can cause segmentation fault in libvirt. (example) % virsh send-key --codeset win32 12 error: End of file while reading data: Input/output error This is caused by overrun at scanning keycode array. Fix it. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> --- src/util/virkeycode.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c index 4d54060..0d42767 100644 --- a/src/util/virkeycode.c +++ b/src/util/virkeycode.c @@ -118,7 +118,7 @@ static int __virKeycodeValueTranslate(unsigned int from_offset, { int i; - for (i = 0; ARRAY_CARDINALITY(virKeycodes); i++) { + for (i = 0; i < ARRAY_CARDINALITY(virKeycodes); i++) { if (getfield(virKeycodes + i, unsigned short, from_offset) == key_value) return getfield(virKeycodes + i, unsigned short, to_offset); } -- 1.7.4.1

2 1

[libvirt] portability issue on Centos 6.0
by Daniel Veillard 22 Aug '11

22 Aug '11

I just tred compiling 0.9.4 and git head on Centos 6.0, for the possible libvirt.org server replacement and it fails in both cases with: CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2161: error: invalid initializer This correspond to the following macro PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); I'm a bit lost how the expansion could lead to this and why, unless the DTrace support in Centos 6 is slightly different than on RHEL, I will look but if someone has an idea :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] How to avoid the disk still persistent after no persistent hotplug and with restarting libvirtd then.
by Osier Yang 22 Aug '11

22 Aug '11

Libvirt loads the domain conf from the status file (/var/run/libvirt/qemu/$dom.xml) if the domain is running, the problem if one restart libvirtd just after some changing on the domain conf, and then destroy/start the guest. libvirt will never known what the original domain conf is then. (vm->newdef is NULL at this moment, generally libvirt restore the domain conf to the original conf (vm->newDef) when destroying and shutdown). Following is a demo of the problem: 1) hotplug a disk to a running domain without "--persistent" specified. 2) restart libvirtd 3) Destroy/start the guest 4) you will see the hotpluged disk is still there. The solution in my mind is to add some attribute to the status XML, such as "live_updated=1", and do the checking when loading the domain conf from status XML: * if "live_updated=1", then loads the domain conf from the persistent domain XML. But the problem is: For a persistent domain, if one made changes on the domain conf both persistently and transiently, we have no way to known which part are changed. Loading from the persistent domain conf is not correct either. For a transient domain, we still have no way to get the original domain conf, that means one will still be surprised when seeing the the disk is still there. Any idea on this? Thanks Osier

2 3

[libvirt] Proposal: A place to store metadata about guests
by Richard W.M. Jones 22 Aug '11

22 Aug '11

I think we need a place to store general metadata about guests. Here are some uses: - Store data about available installer ISOs, guests which are archived and not known to libvirt. - Replace the guest inspection thread in virt-manager. virt-manager would just collect the data from the store. The inspection data would be generated by another process. - Expand on <description/> field in the libvirt XML, and satisfy various requests we've had for a place to store metadata and extend libvirt XML. - Store data on behalf of applications, esp. virt-manager. I'm not envisaging a database or daemon for this. Just a simple directory per host containing flat files. Perhaps we can add some tools or a library to make creation/querying of these files easy. It would be nice if the database was made available over the network (Avahi?). You could also use this database to answer questions like: - "Do I have a Debian wheezy 32 bit guest anywhere on my network?" - "Does any guest have libpng <= <vulnerable-version> installed?" - "Bring up a Fedora 14 instance" (the tools would have to locate a suitable install ISO and put together the correct virt-install command line) There is some crossover with VDSM and even RHEV-M here. Thoughts? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

3 8

[libvirt] [libvirt-snmp][PATCH 1/3] subagent: Avoid dead code
by Michal Privoznik 22 Aug '11

22 Aug '11

Detected by Coverity --- src/libvirtMib_subagent.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/libvirtMib_subagent.c b/src/libvirtMib_subagent.c index f5749be..ee99fb9 100644 --- a/src/libvirtMib_subagent.c +++ b/src/libvirtMib_subagent.c @@ -60,7 +60,7 @@ main (int argc, char **argv) { /* Defs for arg-handling code: handles setting of policy-related variables */ int ch; extern char *optarg; - int dont_fork = 0, use_syslog = 0; + int dont_fork = 0, use_syslog = 1; char *agentx_socket = NULL; while ((ch = getopt(argc, argv, "D:fHLMx:")) != EOF) -- 1.7.3.4

1 2

[libvirt] [PATCH v2] qemu: Allow graceful domain destroy
by Michal Privoznik 22 Aug '11

22 Aug '11

This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable. The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition. --- diff to v1: - more debug printings - don't test (!flags) but (!(flags & VIR_DOMAIN_DESTROY_FLUSH_CACHE)) include/libvirt/libvirt.h.in | 11 ++- src/qemu/qemu_driver.c | 146 ++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 22 ++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 20 ++++++ src/qemu/qemu_monitor_text.h | 1 + tools/virsh.c | 9 ++- 9 files changed, 213 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa29fb6..0610b53 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,13 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ -/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ + +typedef enum { + VIR_DOMAIN_DESTROY_FLUSH_CACHE = 1 << 0, /* In hypervisors supporting this, + try to get cache flushed prior + to destroying the domain */ +} virDomainDestroyFlagsValues; + virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8dda73..44f4bbd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1564,6 +1564,47 @@ cleanup: return ret; } +struct qemuDomainDestroyHelperData { + struct qemud_driver *driver; + virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv; + virMutex lock; + virCond cond; + bool guard; +}; + +static void +qemuDomainDestroyHelper(void *opaque) +{ + struct qemuDomainDestroyHelperData *data = opaque; + struct qemud_driver *driver = data->driver; + virDomainObjPtr vm = data->vm; + qemuDomainObjPrivatePtr priv = data->priv; + + /* Job was already obtained by caller */ + VIR_DEBUG("Destroy thread started, sending 'quit' command."); + qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuMonitorQuit(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + VIR_DEBUG("Command finished."); + + /* To avoid loosing signal, we need to remember + * we tried to send one, but nobody was waiting + * for it. + */ + virMutexLock(&data->lock); + if (data->guard) { + VIR_DEBUG("Driver is listening, signalize on condition"); + virCondSignal(&data->cond); + } else { + VIR_DEBUG("Nobody's listening, setting guard variable"); + data->guard = true; + } + virMutexUnlock(&data->lock); + VIR_DEBUG("Destroy thread exiting"); +} + +#define QEMU_QUIT_WAIT_SECONDS 5 static int qemuDomainDestroyFlags(virDomainPtr dom, @@ -1574,8 +1615,13 @@ qemuDomainDestroyFlags(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + bool use_thread = false; + bool use_kill = false; + virThread destroy_thread; + struct qemuDomainDestroyHelperData data; + unsigned long long then; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_FLUSH_CACHE, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1590,12 +1636,14 @@ qemuDomainDestroyFlags(virDomainPtr dom, priv = vm->privateData; priv->fakeReboot = false; - /* Although qemuProcessStop does this already, there may - * be an outstanding job active. We want to make sure we - * can kill the process even if a job is active. Killing - * it now means the job will be released - */ - qemuProcessKill(vm); + if (!(flags & VIR_DOMAIN_DESTROY_FLUSH_CACHE)) { + /* Although qemuProcessStop does this already, there may + * be an outstanding job active. We want to make sure we + * can kill the process even if a job is active. Killing + * it now means the job will be released + */ + qemuProcessKill(vm); + } if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; @@ -1606,6 +1654,78 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto endjob; } + if (flags & VIR_DOMAIN_DESTROY_FLUSH_CACHE) { + /* Try to shutdown domain gracefully. + * Send 'quit' to qemu. However, qemu can be unresponsive. + * Therefore create a separate thread in which we execute + * that monitor comand. Wait max QEMU_QUIT_WAIT_SECONDS. + */ + VIR_DEBUG("Trying to gracefully shutdown %s", vm->def->name); + if (virMutexInit(&data.lock) < 0) { + virReportOOMError(); + goto endjob; + } + + if (virCondInit(&data.cond) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize thread condition")); + virMutexDestroy(&data.lock); + goto endjob; + } + + data.driver = driver; + data.vm = vm; + data.priv = priv; + data.guard = false; + + VIR_DEBUG("Spawning thread to run 'quit' command"); + if (virThreadCreate(&destroy_thread, true, + qemuDomainDestroyHelper, &data) < 0) { + virReportSystemError(errno, "%s", + _("unable to create destroy thread")); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + goto endjob; + } + use_thread = true; + + if (virTimeMs(&then) < 0) + goto endjob; + + then += QEMU_QUIT_WAIT_SECONDS * 1000; + + /* How synchronization with destroy thread works: + * Basically wait on data.cond. But because conditions + * does not 'remember' that they have been signalized + * data.guard is used. Practically, data.guard says + * to destroy thread if we are waiting on condition + * and to us whether we should even try. + */ + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + VIR_DEBUG("Waiting for thread to exit"); + if (virCondWaitUntil(&data.cond, &data.lock, then) < 0) { + if (errno == ETIMEDOUT) { + VIR_DEBUG("Condition timeout"); + use_kill = true; + data.guard = false; + } else { + virReportSystemError(errno, "%s", _("unable to wait " + "on condition")); + virMutexUnlock(&data.lock); + goto endjob; + } + } + } else { + VIR_DEBUG("Guard variable set, thread exited"); + } + virMutexUnlock(&data.lock); + + if (use_kill) + qemuProcessKill(vm); + } + qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_DESTROYED); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -1626,6 +1746,18 @@ endjob: vm = NULL; cleanup: + if (use_thread) { + VIR_DEBUG("Thread used, clean it up"); + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + ignore_value(virCondWait(&data.cond, &data.lock)); + } + virMutexUnlock(&data.lock); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + virThreadJoin(&destroy_thread); + } if (vm) virDomainObjUnlock(vm); if (event) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..41b9c5c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2475,3 +2475,16 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, ret = qemuMonitorTextBlockJob(mon, device, bandwidth, info, mode); return ret; } + +int qemuMonitorQuit(qemuMonitorPtr mon) +{ + int ret; + + VIR_DEBUG("mon=%p", mon); + + if (mon->json) + ret = qemuMonitorJSONQuit(mon); + else + ret = qemuMonitorTextQuit(mon); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..3fe6bb9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -475,6 +475,8 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorQuit(qemuMonitorPtr mon); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..1f078cd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2956,3 +2956,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONQuit(qemuMonitorPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("quit", NULL); + + if (!cmd) + return -1; + + ret =qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..2a7df76 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -231,4 +231,5 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorJSONQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..ca9c21b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3063,3 +3063,23 @@ cleanup: VIR_FREE(reply); return ret; } + + +int qemuMonitorTextQuit(qemuMonitorPtr mon) +{ + const char *cmd = "quit"; + char *reply = NULL; + int ret = -1; + + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..9ade938 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -224,4 +224,5 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorTextQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_TEXT_H */ diff --git a/tools/virsh.c b/tools/virsh.c index 7d849ec..4c7962b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2567,6 +2567,8 @@ static const vshCmdInfo info_destroy[] = { static const vshCmdOptDef opts_destroy[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"flush", VSH_OT_BOOL, 0, N_("try to flush hypervisor's cache prior to " + "destroying the domain")}, {NULL, 0, 0, NULL} }; @@ -2576,6 +2578,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2583,7 +2586,11 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainDestroy(dom) == 0) { + if (vshCommandOptBool(cmd, "flush")) + flags |= VIR_DOMAIN_DESTROY_FLUSH_CACHE; + + if ((!flags && virDomainDestroy(dom) == 0) || + virDomainDestroyFlags(dom, flags) == 0) { vshPrint(ctl, _("Domain %s destroyed\n"), name); } else { vshError(ctl, _("Failed to destroy domain %s"), name); -- 1.7.3.4

1 0

[libvirt] USB2 controllers & domain XML
by Marc-André Lureau 22 Aug '11

22 Aug '11

Hi I have started looking at: https://bugzilla.redhat.com/show_bug.cgi?id=725670 My initial thought was to just add a new type of controller: <controller type='usb2' index='0'/> But it also needs the 3 companion UHCI controllers (see bug). We can either include them implicitely (not really elegant in libvirt, but doable), or with more flexibility list all the controllers in the XML: <controller type='ich9-usb-ehci1' index='0'/> <controller type='ich9-usb-uhci1' index='0' masterbus='0' firstport='0'> <controller type='ich9-usb-uhci2' index='0' masterbus='0' firstport='2'> <controller type='ich9-usb-uhci3' index='0' masterbus='0' firstport='4'> Any other idea? Daniel, have you also started working on USB2 & usb-redir support? regards -- Marc-André Lureau

5 7

[libvirt] virt-manager, debug error, question
by Jason Helfman 22 Aug '11

22 Aug '11

Hi, I am getting this error when I run virt-manager in FreeBSD: [Fri, 19 Aug 2011 09:32:14 virt-manager 24762] DEBUG (engine:555) No inspection thread because libguestfs is too old, not available, or libvirt is not thread safe. libguestfs isn't ported to FreeBSD, however I was wondering about the thread safe for libvirt. Is there a way to "enable" this, or is the software telling me it really is not thread safe, and can't be. [jhelfman@eggman ~/ports/devel/libvirt/work/libvirt-0.9.4]$ ./configure --help |grep thread --enable-threads={posix|solaris|pth|win32} specify multithreading API --disable-threads build without multithread safety --enable-test-locking thread locking tests using CIL [default=no] Thanks, Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5

3 2

[libvirt] status of ocaml for libvirt
by Jason Helfman 19 Aug '11

19 Aug '11

Hello All, I was looking at porting virt-top to FreeBSD, and noticed that it needed ocaml-libvirt, and I know that isn't ported. What is the status of this library, and it is up-to-date? Thanks, Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5

1 0

[libvirt] [PATCH] virsh: properly interleave shared stdout and stderr
by Eric Blake 19 Aug '11

19 Aug '11

Without this patch, invoking 'virsh >file 2>&1' results in error messages appearing before normal output, even if they occurred later in time than the normal output (since stderr is unbuffered, but stdout waits until a full buffer). * tools/virsh.c (print_job_progress, vshError): Flush between stream transitions. * tests/undefine: Test it. --- This fixes the bug I mentioned here: https://www.redhat.com/archives/libvir-list/2011-August/msg00891.html tests/undefine | 5 +---- tools/virsh.c | 8 ++++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/undefine b/tests/undefine index 6c821ad..22d6c14 100755 --- a/tests/undefine +++ b/tests/undefine @@ -58,17 +58,14 @@ compare exp out || fail=1 # Succeed, now: first shut down, then undefine, both via name. $abs_top_builddir/tools/virsh -q -c test:///default \ - 'shutdown test; undefine test; dominfo test' > out 2> err + 'shutdown test; undefine test; dominfo test' > out 2>&1 test $? = 1 || fail=1 cat <<\EOF > expout || fail=1 Domain test is being shutdown Domain test has been undefined -EOF -cat <<\EOF > experr || fail=1 error: failed to get domain 'test' error: Domain not found EOF compare expout out || fail=1 -compare experr err || fail=1 (exit $fail); exit $fail diff --git a/tools/virsh.c b/tools/virsh.c index 1c67150..7d849ec 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4956,7 +4956,10 @@ print_job_progress(const char *label, unsigned long long remaining, } } + /* see comments in vshError about why we must flush */ + fflush(stdout); fprintf(stderr, "\r%s: [%3d %%]", label, progress); + fflush(stderr); } static bool @@ -14336,6 +14339,10 @@ vshError(vshControl *ctl, const char *format, ...) va_end(ap); } + /* Most output is to stdout, but if someone ran virsh 2>&1, then + * printing to stderr will not interleave correctly with stdout + * unless we flush between every transition between streams. */ + fflush(stdout); fputs(_("error: "), stderr); va_start(ap, format); @@ -14345,6 +14352,7 @@ vshError(vshControl *ctl, const char *format, ...) va_end(ap); fprintf(stderr, "%s\n", NULLSTR(str)); + fflush(stderr); VIR_FREE(str); } -- 1.7.4.4

2 2

[libvirt] Fwd: [virt-tools-list] virt-manager, dependency question
by Jason Helfman 19 Aug '11

19 Aug '11

Any thoughts on this to this list? Figure I would get a quicker response here, possibly. Thanks, Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5

1 0

[libvirt] xml style?
by Eric Blake 19 Aug '11

19 Aug '11

When writing an attribute in xml, which style is preferred? <element attr='single'/> <element attr="double"/> We are currently rather split-brain in libvirt, among three general areas: test programs under tests/*data/*, documentation under docs/*.html.in, and RNG schemas under docs/schemas/*. In all cases, both styles are present, but in different proportions: $ git grep '="' tests/*data |wc 48 163 4276 $ git grep "='" tests/*data |wc 5994 21336 526402 $ git grep '="' docs/schemas/ |wc 1522 4599 103050 $ git grep "='" docs/schemas/ |wc 566 1699 33005 $ git grep '="' docs/*.in |wc 1288 6436 114146 $ git grep "='" docs/*.in |wc 611 2242 42950 Is it worth a cleanup patch that settles on one particular style, or even a syntax check that forces style compliance? -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

1 1

[libvirt] [PATCHv2 1/2] virsh: add virsh snapshot-current --name
by Eric Blake 19 Aug '11

19 Aug '11

Sometimes, full XML is too much; since most snapshot commands operate on a snapshot name, there should be an easy way to get at the current snapshot's name. * tools/virsh.c (cmdSnapshotCurrent): Add an option. * tools/virsh.pod (snapshot-current): Document it. --- v2: use xpath instead of strstr; replaces: https://www.redhat.com/archives/libvir-list/2011-August/msg00329.html tools/virsh.c | 29 ++++++++++++++++++++++++++--- tools/virsh.pod | 4 +++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index bffec1d..fd64020 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12034,6 +12034,7 @@ static const vshCmdInfo info_snapshot_current[] = { static const vshCmdOptDef opts_snapshot_current[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"name", VSH_OT_BOOL, 0, N_("list the name, rather than the full xml")}, {NULL, 0, 0, NULL} }; @@ -12044,6 +12045,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) bool ret = false; int current; virDomainSnapshotPtr snapshot = NULL; + char *xml = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -12056,7 +12058,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (current < 0) goto cleanup; else if (current) { - char *xml; + char *name = NULL; if (!(snapshot = virDomainSnapshotCurrent(dom, 0))) goto cleanup; @@ -12065,13 +12067,34 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (!xml) goto cleanup; - vshPrint(ctl, "%s", xml); - VIR_FREE(xml); + if (vshCommandOptBool(cmd, "name")) { + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + + xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", + NULL, XML_PARSE_NOENT | XML_PARSE_NONET | + XML_PARSE_NOWARNING); + if (!xmldoc) + goto cleanup; + ctxt = xmlXPathNewContext(xmldoc); + if (!ctxt) { + xmlFreeDoc(xmldoc); + } + + name = virXPathString("string(/domainsnapshot/name)", ctxt); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + if (!name) + goto cleanup; + } + + vshPrint(ctl, "%s", name ? name : xml); } ret = true; cleanup: + VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); if (dom) diff --git a/tools/virsh.pod b/tools/virsh.pod index ec778bf..53376d6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1569,9 +1569,11 @@ Create a snapshot for domain I<domain> with the given <name> and value. If I<--print-xml> is specified, then XML appropriate for I<snapshot-create> is output, rather than actually creating a snapshot. -=item B<snapshot-current> I<domain> +=item B<snapshot-current> I<domain> [I<--name>] Output the snapshot XML for the domain's current snapshot (if any). +If I<--name> is specified, just list the snapshot name instead of the +full xml. =item B<snapshot-list> I<domain> -- 1.7.4.4

2 6

[libvirt] [PATCH 0/3] xml simplifications
by Eric Blake 19 Aug '11

19 Aug '11

I noticed this while working on my snapshot stuff. There was lots of copy and paste going on in virsh.c, and it felt awful to not be able to represent the same idea in fewer lines, so I fixed it. This will probably mean I have to rebase the entire snapshot series, since it will introduce several conflicts in at least virsh.c; oh well. Eric Blake (3): maint: treat more libxml2 functions as free-like xml: add another convenience function maint: simplify lots of libxml2 clients cfg.mk | 2 + src/conf/domain_conf.c | 20 +------ src/conf/storage_conf.c | 8 +--- src/cpu/cpu.c | 18 +----- src/esx/esx_vi.c | 17 ++----- src/libvirt_private.syms | 2 - src/qemu/qemu_migration.c | 9 +--- src/security/virt-aa-helper.c | 12 +---- src/test/test_driver.c | 13 +---- src/util/xml.c | 53 +++++------------- src/util/xml.h | 84 +++++++++++++++++++++++++--- tests/cputest.c | 9 +--- tools/virsh.c | 120 +++++++++-------------------------------- 13 files changed, 135 insertions(+), 232 deletions(-) -- 1.7.4.4

2 9

[libvirt] Allow to undefine a running domain
by Osier Yang 19 Aug '11

19 Aug '11

Per API virDomainUndefineFlags' doc says, if a domain is running, it will be converted to transient, but keep running. However, the drivers prohibit one undefining a running domain. This patch series modify the internal domainUndefineFlags function of all drivers (except some driver don't need to check if the domain is running, it's handled by the underlying hypervisor, such as ESX and XEND). The principle is: 1) Set vm->persistent = 0 for a running domain 2) domainDestroy and domainShutdown will take care of remove the domain obj from the hash table. [PATCH 1/8] libxl: Allow to undefine a running domain [PATCH 2/8] lxc: Allow to undefine a running domain [PATCH 3/8] openvz: Allow to undefine a running domain [PATCH 4/8] qemu: Allow to undefine a running domain [PATCH 5/8] test: Allow to undefine a running domain [PATCH 6/8] uml: Allow to undefine a running domain [PATCH 7/8] vmware: Allow to undefine a running domain [PATCH 8/8] xen: Allow to undefine a running domain (xm_internal) Split the patches for easy reviewing, will merge when committing if need. Regards Osier

2 28

[libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
by Osier Yang 19 Aug '11

19 Aug '11

* src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as "balloon: actual=", which cause "actual=" is stripped early before the real parsing. This patch changes BALLOON_PREFIX into "balloon: ", and modifies related functions, also renames "qemuMonitorParseExtraBalloonInfo" to "qemuMonitorParseBalloonInfo", as after the changing, it parses all the info returned by "info balloon". --- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++---------------- 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..d17d863 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned int tag, return 0; } - /* Convert bytes to kilobytes for libvirt */ switch (tag) { + /* Convert megabytes to kilobytes for libvirt */ + case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: + value = value << 10; + break; + /* Convert bytes to kilobytes for libvirt */ case VIR_DOMAIN_MEMORY_STAT_SWAP_IN: case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT: case VIR_DOMAIN_MEMORY_STAT_UNUSED: @@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag, /* The reply from the 'info balloon' command may contain additional memory * statistics in the form: '[,<tag>=<val>]*' */ -static int qemuMonitorParseExtraBalloonInfo(char *text, - virDomainMemoryStatPtr stats, - unsigned int nr_stats) +static int qemuMonitorParseBalloonInfo(char *text, + virDomainMemoryStatPtr stats, + unsigned int nr_stats) { char *p = text; unsigned int nr_stats_found = 0; while (*p && nr_stats_found < nr_stats) { - if (parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, + if (parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, + "actual=", &stats[nr_stats_found]) || + parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, ",mem_swapped_in=", &stats[nr_stats_found]) || parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, ",mem_swapped_out=", &stats[nr_stats_found]) || @@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_UNUSED, ",free_mem=", &stats[nr_stats_found]) || parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE, - ",total_mem=", &stats[nr_stats_found]) || - parseMemoryStat(&p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, - ",actual=", &stats[nr_stats_found])) + ",total_mem=", &stats[nr_stats_found])) nr_stats_found++; /* Skip to the next label. When *p is ',' the last match attempt @@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ -#define BALLOON_PREFIX "balloon: actual=" +#define BALLOON_PREFIX "balloon: " /* * Returns: 0 if balloon not supported, +1 if balloon query worked @@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned int memMB; char *end; offset += strlen(BALLOON_PREFIX); - if (virStrToLong_ui(offset, &end, 10, &memMB) < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not parse memory balloon allocation from '%s'"), reply); - goto cleanup; + + if (STRPREFIX(offset, "actual=")) { + offset += strlen("actual="); + + if (virStrToLong_ui(offset, &end, 10, &memMB) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("could not parse memory balloon allocation from '%s'"), reply); + goto cleanup; + } + *currmem = memMB * 1024; + ret = 1; + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected balloon information '%s'"), reply); + ret = -1; } - *currmem = memMB * 1024; - ret = 1; } else { /* We don't raise an error here, since its to be expected that * many QEMU's don't support ballooning @@ -660,9 +673,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) { offset += strlen(BALLOON_PREFIX); - if ((offset = strchr(offset, ',')) != NULL) { - ret = qemuMonitorParseExtraBalloonInfo(offset, stats, nr_stats); - } + ret = qemuMonitorParseBalloonInfo(offset, stats, nr_stats); } VIR_FREE(reply); -- 1.7.6

4 11

[libvirt] [PATCH] build: fix compilation on mingw64
by Eric Blake 19 Aug '11

19 Aug '11

* .gnulib: Update to latest, for getcwd fixes. --- Spurred by a gnulib bug report by Marc-André Lureau. Fedora currently uses the mingw32 cross-compiler, but is aiming to switch to the mingw64 cross-compiler; once that is done, failure to upgrade gnulib would trip up cross-compilation during ./autobuild.sh. * .gnulib 4470580...56d6664 (21): > getcwd: fix test failures on mingw > getcwd-lgpl: fix m4 to match relaxed test for BSD > getcwd: fix compilation on mingw64 > pipe2: silence compiler warning > autoupdate > relocatable-prog: fix link error > getaddrinfo: fix sh typo in gai_strerrorA decl checking > base64: Reference latest RFC. > * build-aux/bootstrap (slurp): Remove obsolescent gettext.m4 patch. > po/Makefile.in.in: fix make -q problem > configmake: fix make -q problem > git-version-gen: correct the advice in a comment > base64: fix off-by-one buffer size bug > closein: correct comments > More tests for 'fseeko'. > ChangeLog: fix wrong attribution in last commit > fseeko: remove unneeded hack > fseeko: fix bug on glibc > unictype/base: Fix interoperability with preinstalled libunistring. > iswblank: Detect declaration correctly. > tcgetsid: Detect declaration correctly. .gnulib | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/.gnulib b/.gnulib index 4470580..56d6664 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 4470580881a7b821b52fb5635102ef3e27aa5af4 +Subproject commit 56d6664559f449af25f0d331457b014b02324d65 -- 1.7.4.4

2 2

[libvirt] [RFC] Add a new feild to qemud_save_header to tell if the saved image is corrupt
by Osier Yang 19 Aug '11

19 Aug '11

If one tries to restore a domain from a corrupt save image, we blindly goes forward to restore from it, this can cause many different errors, depending on how much the image is saved. E.g. https://bugzilla.redhat.com/show_bug.cgi?id=730750 So I'm thinking if we can introduce a new feild to struct qemud_save_header, such as "bool complete;", and set it true if succeeded to save the image, false if not. So that could do some checking while trying to open the image (qemuDomainSaveImageOpen), and quit early if "complete" is false, with a sensiable error message. Thought? Thanks Osier

3 9

[libvirt] [PATCH] storage: Flush host cache after write
by Michal Privoznik 19 Aug '11

19 Aug '11

Although we are flushing cache after some critical writes (e.g. volume creation), after some others we do not (e.g. volume cloning). This patch fix this issue. That is for volume cloning, writing header of logical volume, and storage wipe. --- src/storage/storage_backend.c | 8 ++++++++ src/storage/storage_backend_logical.c | 7 +++++++ src/storage/storage_driver.c | 8 ++++++++ 3 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 6243d1e..889f530 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -208,6 +208,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, } while ((amtleft -= interval) > 0); } + if (fdatasync(fd) < 0) { + ret = -errno; + virReportSystemError(errno, _("cannot sync data to file '%s'"), + vol->target.path); + goto cleanup; + } + + if (VIR_CLOSE(inputfd) < 0) { ret = -errno; virReportSystemError(errno, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index c622d2a..ca4166d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -424,6 +424,13 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_FORCE_CLOSE(fd); goto cleanup; } + if (fsync(fd) < 0) { + virReportSystemError(errno, + _("cannot flush header of device'%s'"), + pool->def->source.devices[i].path); + VIR_FORCE_CLOSE(fd); + goto cleanup; + } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot close device '%s'"), diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6715790..68cac1f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1777,6 +1777,14 @@ storageWipeExtent(virStorageVolDefPtr vol, remaining -= written; } + if (fdatasync(fd) < 0) { + ret = -errno; + virReportSystemError(errno, + _("cannot sync data to volume with path '%s'"), + vol->target.path); + goto out; + } + VIR_DEBUG("Wrote %zu bytes to volume with path '%s'", *bytes_wiped, vol->target.path); -- 1.7.3.4

3 4

[libvirt] [PATCH v2] daemon: initialize GnuTLS
by Michal Privoznik 19 Aug '11

19 Aug '11

When spice_tls is set but listen_tls is not, we don't initialize GnuTLS library. So any later gnutls call (e.g. during migration, where we initialize a certificate) will access uninitialized GnuTLS internal structs and throws an error. Although, we might now initialize GnuTLS twice, it is safe according to the documentation: This function can be called many times, but will only do something the first time. This patch creates 2 functions: virNetTLSInit and virNetTLSDeinit with respect to written above. --- diff to v1: - moved from qemu to daemon - created special init function daemon/libvirtd.c | 2 ++ src/rpc/virnettlscontext.c | 25 ++++++++++++++++++++++--- src/rpc/virnettlscontext.h | 3 +++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b99c637..0530ba5 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1516,6 +1516,7 @@ int main(int argc, char **argv) { virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START, 0, "start", NULL); + virNetTLSInit(); if (daemonSetupNetworking(srv, config, sock_file, sock_file_ro, ipsock, privileged) < 0) { @@ -1554,6 +1555,7 @@ cleanup: virNetServerProgramFree(qemuProgram); virNetServerClose(srv); virNetServerFree(srv); + virNetTLSDeinit(); if (statuswrite != -1) { if (ret != 0) { /* Tell parent of daemon what failed */ diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 19a9b25..8482eaf 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -679,9 +679,6 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, ctxt->refs = 1; - /* Initialise GnuTLS. */ - gnutls_global_init(); - if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) { int val; if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0) @@ -1399,3 +1396,25 @@ void virNetTLSSessionFree(virNetTLSSessionPtr sess) virMutexDestroy(&sess->lock); VIR_FREE(sess); } + +/* + * This function MUST be called before any + * virNetTLS* because it initializes + * underlying GnuTLS library. According to + * it's documentation, it's safe to be called + * many times, but is not thread safe. Each + * call SHOULD be later followed by + * virNetTLSContextDeinit. + */ +void virNetTLSInit(void) +{ + gnutls_global_init(); +} + +/* + * See virNetTLSInit + */ +void virNetTLSDeinit(void) +{ + gnutls_global_deinit(); +} diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 641d67e..99f31b9 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -30,6 +30,9 @@ typedef struct _virNetTLSSession virNetTLSSession; typedef virNetTLSSession *virNetTLSSessionPtr; +void virNetTLSInit(void); +void virNetTLSDeinit(void); + virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath, bool tryUserPkiPath, const char *const*x509dnWhitelist, -- 1.7.3.4

2 2

[libvirt] [PATCH] schedinfo: add missing documentation
by Taku Izumi 19 Aug '11

19 Aug '11

This patch adds the missing documentation about the scheduler parameter "vcpu_period" and "vcpu_quota". Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com> --- tools/virsh.pod | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -712,7 +712,9 @@ I<domain-id> Allows you to show (and set) the domain scheduler parameters. The parameters available for each hypervisor are: -LXC, QEMU/KVM (posix scheduler): cpu_shares +LXC (posix scheduler) : cpu_shares + +QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota Xen (credit scheduler): weight, cap @@ -729,6 +731,10 @@ Therefore, -1 is a useful shorthand for B<Note>: The weight and cap parameters are defined only for the XEN_CREDIT scheduler and are now I<DEPRECATED>. +B<Note>: The vcpu_period parameter has a valid value range of 1000-1000000 or +0, and the vcpu_quota parameter has a valid value range of 1000-1844674407370955 +or less than 0. + =item B<screenshot> I<domain-id> [I<imagefilepath>] [I<--screen> B<screenID>] Takes a screenshot of a current domain console and stores it into a file.

2 1

[libvirt] [PATCH] maint: ignore generated file
by Eric Blake 18 Aug '11

18 Aug '11

I did 'git add .' while in the middle of 'make syntax-check', and it picked up a temporary file that should not be committed. * .gitignore: Ignore sc_* from syntax check. --- Pushing under the trivial rule. .gitignore | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index e8b2dbf..39ecd6d 100644 --- a/.gitignore +++ b/.gitignore @@ -53,6 +53,7 @@ /po/* /proxy/ /python/generator.py.stamp +/sc_* /src/libvirt_iohelper /src/locking/qemu-sanlock.conf /src/remote/*_client_bodies.h -- 1.7.4.4

1 0

[libvirt] [RFC] Should libvirt be a proxy for migration data
by Jiri Denemark 18 Aug '11

18 Aug '11

Hi all, Currently when we start a non-tunneled migration, data go straight from source qemu to destination qemu. This is nice in that there is no additional overhead but it also has several disadvantages. If the communication between source and destination qemu breaks, we only get unexpected error message from qemu with no glue about what happened. Another issue is that if qemu cannot send migration data, we cannot cancel the migration because migrate_cancel blocks until all buffers with migration data queued up for transmission are written into the socket. That said, I think we should act as a proxy between source and destination qemu so that we can detect and report normal errors (such as connection reset by peer) and cancel migration at any time. Since we have virNetSocket and we already use that for connecting to destination qemu, we should use it for proxying migration data as well. This approach also has some disadvantages, e.g., a single libvirt thread instead of several qemu processes will now send migration data from all domains that are being migrated. However, I feel like the gain is bigger than the downside. And we already do the same for tunneled migration anyway. Any objections? Jirka

2 2

Re: [libvirt] [Qemu-devel] Killing block migration in qemu?
by Anthony Liguori 17 Aug '11

17 Aug '11

On 08/17/2011 11:51 AM, Paolo Bonzini wrote: > Hi all, > > following discussions yesterday with Juan Quintela and Marcelo Tosatti, > here is my humble proposal: remove block migration from qemu master. +1 I was going to propose the same thing after the conference actually. Regards, Anthony Liguori

1 0

[libvirt] [PATCH 0/5 v2] Some rearrangement & additions to pid file handling/locking
by Daniel P. Berrange 17 Aug '11

17 Aug '11

An update to: https://www.redhat.com/archives/libvir-list/2011-August/msg00349.html Changes in v2: - Fix all docs & code bugs from previous review

4 13

[libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
by Adam Litke 17 Aug '11

17 Aug '11

Here are the patches to implement the BlockPull/BlockJob API as discussed and agreed to. I am testing with a python script (included for completeness as the final patch). The qemu monitor interface is not expected to change in the future. Stefan is planning to submit placeholder commands for upstream qemu until the generic streaming support is implemented. Changes since V1: - Make virDomainBlockPullAbort() and virDomainGetBlockPullInfo() into a generic BlockJob interface. - Added virDomainBlockJobSetSpeed() - Rename VIR_DOMAIN_EVENT_ID_BLOCK_PULL event to fit into block job API - Add bandwidth argument to virDomainBlockPull() Summary of changes since first generation patch series: - Qemu dropped incremental streaming so remove libvirt incremental BlockPull() API - Rename virDomainBlockPullAll() to virDomainBlockPull() - Changes required to qemu monitor handlers for changed command names -- To help speed the provisioning process for large domains, new QED disks are created with backing to a template image. These disks are configured with copy on read such that blocks that are read from the backing file are copied to the new disk. This reduces I/O over a potentially costly path to the backing image. In such a configuration, there is a desire to remove the dependency on the backing image as the domain runs. To accomplish this, qemu will provide an interface to perform sequential copy on read operations during normal VM operation. Once all data has been copied, the disk image's link to the backing file is removed. The virDomainBlockPull API family brings this functionality to libvirt. virDomainBlockPull() instructs the hypervisor to stream the entire device in the background. Progress of this operation can be checked with the function virDomainBlockJobInfo(). An ongoing stream can be cancelled with virDomainBlockJobAbort(). virDomainBlockJobSetSpeed() allows you to limit the bandwidth that the operation may consume. An event (VIR_DOMAIN_EVENT_ID_BLOCK_JOB) will be emitted when a disk has been fully populated or if a BlockPull() operation was terminated due to an error. This event is useful to avoid polling on virDomainBlockJobInfo() for completion and could also be used by the security driver to revoke access to the backing file when it is no longer needed. make check: PASS make syntax-check: PASS make -C tests valgrind: PASS [PATCH 1/8] Add new API virDomainBlockPull* to headers [PATCH 2/8] virDomainBlockPull: Implement the main entry points [PATCH 3/8] Add virDomainBlockPull support to the remote driver [PATCH 4/8] Implement virDomainBlockPull for the qemu driver [PATCH 5/8] Enable the virDomainBlockPull API in virsh [PATCH 6/8] Enable virDomainBlockPull in the python API. [PATCH 7/8] Asynchronous event for BlockJob completion [PATCH 8/8] Test the blockJob/BlockPull API

6 22

Re: [libvirt] [Qemu-devel] Killing block migration in qemu?
by Stefan Hajnoczi 17 Aug '11

17 Aug '11

On Wed, Aug 17, 2011 at 5:51 PM, Paolo Bonzini <pbonzini(a)redhat.com> wrote: > following discussions yesterday with Juan Quintela and Marcelo Tosatti, here > is my humble proposal: remove block migration from qemu master. It seems to > me that keeping block migration is going to slow down further improvements > on migration. The main problems are: > > 1) there are very good reasons to move migration to a separate thread. Only > a limited amount of extra locking, perhaps none is needed in order to do so > for RAM and devices. But the block drivers pretty much need to run under > the I/O thread lock, and coroutines will not help if the I/O thread is taken > by another thread. It's hard/unreliable/pointless to ping-pong migration > between threads. The image streaming approach will also run in the I/O thread for the mid-term future. Is the problem that the block migration code today is too tied into the actual migration code path and therefore stops from using it when migration happens in a separate thread? > > 2) there already are plans to reimplement block migration... it's called > streaming :) and not coincidentially it reuses some of the block migration > code. What are the concrete issues with the existing block migration code? I'm not disagreeing that we should move to a streaming approach but I simply don't know the details of the existing block migration code. > Here is how it would go: This sounds reasonable. In fact we can do both pre-copy and post-copy block migration using streaming (+mirroring). Stefan

2 1

[libvirt] [PATCH] qemu: Initialize GnuTLS
by Michal Privoznik 17 Aug '11

17 Aug '11

When spice_tls is set but listen_tls is not, we don't initialize GnuTLS library. So any later gnutls call (e.g. during migration, where we initialize a certificate) will access uninitialized GnuTLS internal structs and throws an error. Although, we might now initialize GnuTLS twice, it is safe according to the documentation: This function can be called many times, but will only do something the first time. --- src/qemu/qemu_driver.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..5fe20b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -44,6 +44,7 @@ #include <sys/ioctl.h> #include <sys/un.h> #include <byteswap.h> +#include <gnutls/gnutls.h> #include "qemu_driver.h" @@ -537,6 +538,15 @@ qemudStartup(int privileged) { } VIR_FREE(driverConf); + if (qemu_driver->spiceTLS) { + /* Initialize GnuTLS. If it was initialized before, + * it doesn't hurt. From GnuTLS documentation: + * This function can be called many times, + * but will only do something the first time. + */ + gnutls_global_init(); + } + /* We should always at least have the 'nop' manager, so * NULLs here are a fatal error */ @@ -754,6 +764,9 @@ qemudShutdown(void) { qemuProcessAutoDestroyShutdown(qemu_driver); + if (qemu_driver->spiceTLS) + gnutls_global_deinit(); + VIR_FREE(qemu_driver->configDir); VIR_FREE(qemu_driver->autostartDir); VIR_FREE(qemu_driver->logDir); -- 1.7.3.4

2 1

[libvirt] [PATCH 0/2] Libvirt-java event handling
by js＠alien8.de 17 Aug '11

17 Aug '11

From: Julian Stecklina <jsteckli(a)os.inf.tu-dresden.de> I noticed that event handling in the libvirt Java bindings is incomplete. The first patch adds required data types and constants. The second relaxes the visibility of the Domain constructor, otherwise Callbacks are kind of useless. Regards, Julian Julian Stecklina (2): Allow different types of callbacks (just as the C API). Allow to construct Domain objects from inside callbacks. src/main/java/org/libvirt/Connect.java | 3 ++- src/main/java/org/libvirt/Domain.java | 2 +- src/main/java/org/libvirt/jna/Libvirt.java | 22 ++++++++++++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) -- 1.7.3.4

3 4

Re: [libvirt] patch for disk migration verbose progress
by Eric Blake 17 Aug '11

17 Aug '11

[re-adding the list] On 08/17/2011 12:30 AM, Tom Vijlbrief wrote: > Hi Eric > > This is the complete patch including json (not tested) and a typo correction > in an error message... > > Tom > ---------- Doorgestuurd bericht ---------- > Van: "Tom Vijlbrief"<tom(a)nomadbsd.v7f.eu> > Datum: 17 aug. 2011 08:23 > Onderwerp: mon.patch > Aan:<tvijlbrief(a)gmail.com> > > > mon.patch > Thanks again for taking the time to contribute. It is easier to work with patches generated by 'git format-patch' (and 'git send-email' is a nice wrapper that both formats the patch and mails it to the list); as written, your patch had no commit message, so I had to invent one. I also added you to AUTHORS; let me know if I need to update any preferred spellings. > > # NB, must pass the --listen flag to the libvirtd process for this to Is this a stray line in your email, or is it pointing out a further issue? > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 2a9a078..618a45c 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -1894,6 +1894,31 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, > _("migration was active, but RAM 'total' data was missing")); > return -1; > } > + > + virJSONValuePtr disk = virJSONValueObjectGet(ret, "disk"); > + if (!disk) { > + return 0; > + } > + > + unsigned long long t; Our style tends to float declarations to the top, C89 style, even though this is valid as written (we require C99 for other reasons). > + if (virJSONValueObjectGetNumberUlong(disk, "transferred",&t)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("disk migration was active, but DISK 'transferred' data was missing")); > + return -1; > + } > + (*transferred)+= t; > + if (virJSONValueObjectGetNumberUlong(disk, "remaining",&t)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("disk migration was active, but DISK 'remaining' data was missing")); > + return -1; > + } > + (*remaining)+= t; I tweaked spacing between operators (yeah, it doesn't help that thunderbird corrupted spacing even worse in my reply to your mail). And the () are redundant; this can safely be: *remaining += t; > + if (virJSONValueObjectGetNumberUlong(disk, "total",&t)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("disk migration was active, but DISK 'total' data was missing")); > + return -1; > + } > + (*total)+= t; > } > > return 0; > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c > index 7bf733d..fef6f36 100644 > --- a/src/qemu/qemu_monitor_text.c > +++ b/src/qemu/qemu_monitor_text.c > @@ -149,7 +149,7 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, > passwd = strstr(start, PASSWORD_PROMPT); > if (passwd) { > #if DEBUG_IO > - VIR_DEBUG("Seen a passwowrd prompt [%s]", data + used); > + VIR_DEBUG("Seen a password prompt [%s]", data + used); > #endif > if (msg->passwordHandler) { > int i; > @@ -1183,6 +1183,9 @@ cleanup: > #define MIGRATION_TRANSFER_PREFIX "transferred ram: " > #define MIGRATION_REMAINING_PREFIX "remaining ram: " > #define MIGRATION_TOTAL_PREFIX "total ram: " > +#define MIGRATION_DISK_TRANSFER_PREFIX "transferred disk: " > +#define MIGRATION_DISK_REMAINING_PREFIX "remaining disk: " > +#define MIGRATION_DISK_TOTAL_PREFIX "total disk: " > > int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, > int *status, > @@ -1246,6 +1249,7 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, > goto cleanup; > } > *remaining *= 1024; > + tmp = end; > > if (!(tmp = strstr(tmp, MIGRATION_TOTAL_PREFIX))) > goto done; > @@ -1257,7 +1261,53 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, > goto cleanup; > } > *total *= 1024; > + tmp = end; > + > + /* > + * Check for Optional Disk Migration status > + */ > + > + unsigned long long disk_transferred= 0; > + unsigned long long disk_remaining= 0; > + unsigned long long disk_total= 0; > + > + if (!(tmp = strstr(tmp, MIGRATION_DISK_TRANSFER_PREFIX))) > + goto done; > + tmp += strlen(MIGRATION_DISK_TRANSFER_PREFIX); > > + if (virStrToLong_ull(tmp,&end, 10,&disk_transferred)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse disk migration data transferred statistic %s"), tmp); > + goto cleanup; > + } > + disk_transferred *= 1024; > + (*transferred)+= disk_transferred; For one less assignment, I merged these two lines into: *transferred += disk_transferred * 1024; > + tmp = end; > + > + if (!(tmp = strstr(tmp, MIGRATION_DISK_REMAINING_PREFIX))) > + goto done; > + tmp += strlen(MIGRATION_DISK_REMAINING_PREFIX); > + > + if (virStrToLong_ull(tmp,&end, 10,&disk_remaining)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse disk migration data remaining statistic %s"), tmp); > + goto cleanup; > + } > + disk_remaining *= 1024; > + (*remaining)+= disk_remaining; > + tmp = end; > + > + if (!(tmp = strstr(tmp, MIGRATION_DISK_TOTAL_PREFIX))) > + goto done; > + tmp += strlen(MIGRATION_DISK_TOTAL_PREFIX); > + > + if (virStrToLong_ull(tmp,&end, 10,&disk_total)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse disk migration data total statistic %s"), tmp); > + goto cleanup; > + } > + disk_total *= 1024; > + (*total)+= disk_total; > } > } ACK and pushed with those tweaks. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

1 0

Re: [libvirt] Patch for verbose Disk transfer
by Eric Blake 17 Aug '11

17 Aug '11

[re-adding the list] On 08/16/2011 11:52 PM, Tom Vijlbrief wrote: > Hi Eric, > > Yes json has similar info and I also patched it, but I don't know how to > configure vir-sh so that it uses the json channel for at least testing the > patch. I can send you the patch anyway, it's a simple change... Use of JSON depends on which version of qemu you are running; if you are using Fedora 15 or newer, then JSON should automatically be in use. It's not something you can configure at the virsh command line. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

1 0

[libvirt] [PATCH] qemu: Init reattaching related members of pciDevice before reattach
by Osier Yang 17 Aug '11

17 Aug '11

Otherwise the device will still be bound to pci-stub driver even it's set as "managed=yes" when do detaching. Of course, it won't triger any driver reprobing too. --- src/qemu/qemu_hotplug.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5f449fb..b7fdfa0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1761,6 +1761,7 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, pciDeviceListDel(driver->activePciHostdevs, pci); if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) ret = -1; + pciDeviceReAttachInit(pci); qemuReattachPciDevice(pci, driver); pciFreeDevice(pci); } -- 1.7.6

2 2

[libvirt] [PATCH] conf: Do not load domain if the domain with same already exists
by Osier Yang 17 Aug '11

17 Aug '11

We don't allow to define domain with same name and different UUID, or with same UUID, so it's reasonable to not load the domain config if there is domain with same name already exists. Otherwise it can cause problem like: 1) % cp /etc/libvirt/qemu/dom.xml /etc/libvirt/qemu/dom_diffuuid.xml 2) remove the line with "uuid" in the "dom_diffuuid.xml" 3) % for i in {1..10}; do kill -SIGHUP $(pidof libvirtd); done 4) % virsh list --all There will be 11 domains listed with the same name, as if there is no UUID specified in domain XML, libvirt will generate one for it, which will be definitely different with the original one. --- src/conf/domain_conf.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce1f3c5..0bed929 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10535,7 +10535,8 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, /* if the domain is already in our hashtable, we only need to * update the autostart flag */ - if ((dom = virDomainFindByUUID(doms, def->uuid))) { + if ((dom = virDomainFindByUUID(doms, def->uuid)) || + (dom = virDomainFindByName(doms, def->name))) { dom->autostart = autostart; VIR_FREE(configFile); -- 1.7.6

2 2

[libvirt] [PATCH] Ensure stream is aborted when exiting console
by Daniel P. Berrange 17 Aug '11

17 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> After running 'virsh console' in interactive mode, there was a missing call to virStreamAbort, which meant the server kept the stream resources open * tools/console.c: Abort stream when exiting --- tools/console.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/console.c b/tools/console.c index 171ebc9..82ef537 100644 --- a/tools/console.c +++ b/tools/console.c @@ -90,9 +90,11 @@ static void virConsoleShutdown(virConsolePtr con) { con->quit = true; - virStreamEventRemoveCallback(con->st); - if (con->st) + if (con->st) { + virStreamEventRemoveCallback(con->st); + virStreamAbort(con->st); virStreamFree(con->st); + } if (con->stdinWatch != -1) virEventRemoveHandle(con->stdinWatch); if (con->stdoutWatch != -1) -- 1.7.6

3 6

[libvirt] [PATCH] Ensure async packets never get marked for sync replies
by Daniel P. Berrange 16 Aug '11

16 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> If a client had initiated a stream abort, it will have a call waiting for a reply in the queue. If more data continues to arrive on the stream, the abort command could mistakenly get signalled as complete. Remove the code from async data processing that looked for waiting calls. Add a sanity check to ensure no async call can ever be marked as needing a reply * src/rpc/virnetclient.c: Ensure async data packets can't trigger a reply --- src/rpc/virnetclient.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index efaac88..055361d 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -627,11 +627,6 @@ static int virNetClientCallDispatchStream(virNetClientPtr client) case VIR_NET_CONTINUE: { if (virNetClientStreamQueuePacket(st, &client->msg) < 0) return -1; - - if (thecall && thecall->expectReply) { - VIR_DEBUG("Got sync data packet completion"); - thecall->mode = VIR_NET_CLIENT_MODE_COMPLETE; - } return 0; } @@ -1193,6 +1188,13 @@ int virNetClientSend(virNetClientPtr client, virNetClientCallPtr call; int ret = -1; + if (expectReply && + (msg->header.status == VIR_NET_CONTINUE)) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Attempt to send an asynchronous message with a synchronous reply")); + return -1; + } + if (VIR_ALLOC(call) < 0) { virReportOOMError(); return -1; -- 1.7.6

2 2

[libvirt] [PATCH] Don't attempt to read from a stream if it is closed
by Daniel P. Berrange 16 Aug '11

16 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The I/O event callback processes incoming packets first, and then does outgoing packets. If the incoming packet caused the stream to close, then the attempt to process outgoing data resulted in an error. This caused libvirt to then send an error back to the client, but the stream had already been stopped. This confused the client since it sees 2 error events. * daemon/stream.c: Don't attempt read if stream is closed --- daemon/stream.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index 7dd9ae7..7d2b367 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -678,7 +678,15 @@ daemonStreamHandleRead(virNetServerClientPtr client, size_t bufferLen = VIR_NET_MESSAGE_PAYLOAD_MAX; int ret; - VIR_DEBUG("client=%p, stream=%p", client, stream); + VIR_DEBUG("client=%p, stream=%p tx=%d closed=%d", + client, stream, stream->tx, stream->closed); + + /* We might have had an event pending before we shut + * down the stream, so if we're marked as closed, + * then do nothing + */ + if (stream->closed) + return 0; /* Shouldn't ever be called unless we're marked able to * transmit, but doesn't hurt to check */ -- 1.7.6

2 1

[libvirt] [RFC] Host CPU passthrough
by Jiri Denemark 16 Aug '11

16 Aug '11

Hi, AFAIK this topic is not new but I think we still do not have a good solution for it. Libvirt already supports specifying what CPU and its features a guest should see but imagine one wants to run a guest on the best possible CPU. The current way is to copy the <cpu> element from capabilities XML into domain XML. This approach is fine since it provides reproducible environment and such domain can even be migrated to a different host. But the CPU shown provided to a guest is not the same as the real host CPU. It's based on a model which doesn't reflect all aspects of real CPUs. Ideally, we would model everything but that's quite complicated and may need updating anytime a new CPU is introduced. I think we should add a possibility to passthrough host CPU into a domain no matter what the CPU looks like. Domains configured in such a way will not be allowed to be migrated since we cannot guarantee the same CPU on destination host but they would be able to see the real CPU. Another benefit is that a single XML configuration would work on any CPU without modifications. Since possible issues with such domains will be harder to debug we can mark them as tainted just like we do if someone calls qemu monitor commands directly. I'm not sure what XML representation makes the most sense so I'll mention several options which I'm thinking about: - "host" ("native", "passthrough", whatever) special model; same name space as real CPU models, I don't like this too much: <cpu match='exact'> <model>host</model> </cpu> - "host" match policy; has nice first-look semantics that the guest CPU matches host's but doesn't play very nice with the defined semantics of "match" attribute: <cpu match='host'/> - new "mode" attribute; IMHO the best one, similar to what we have for SMBIOS: <cpu mode='host'/> vs. <cpu mode='custom' match='...'> <model>...</model. ... </cpu> What do you think? Jirka

3 6

[libvirt] [PATCH] snapshot: tweak misleading wording
by Eric Blake 16 Aug '11

16 Aug '11

Fixes confusion introduced in commit 98369d3. * tools/virsh.c (cmdSnapshotParent): Operates on named snapshot, not current. --- virsh.pod got it right. I'm pushing this under the trivial rule. tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 51ba0a8..49917f3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12282,7 +12282,7 @@ cleanup: * "snapshot-parent" command */ static const vshCmdInfo info_snapshot_parent[] = { - {"help", N_("Get the name of the parent of the current snapshot")}, + {"help", N_("Get the name of the parent of a snapshot")}, {"desc", N_("Extract the snapshot's parent, if any")}, {NULL, NULL} }; -- 1.7.4.4

1 0

[libvirt] main: fix some compilation issues on non-linux platforms
by Stefan Berger 16 Aug '11

16 Aug '11

This patch fixes *some* compilation issues on non-Linux platforms (cygwin). Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> diff --git a/src/util/interface.c b/src/util/interface.c index aec12f5..5d473b7 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -1317,7 +1317,7 @@ ifaceGetPhysicalFunction(const char *ifname, char **pfname) } #else int -ifaceIsVirtualFunction(const char *ifname) +ifaceIsVirtualFunction(const char *ifname ATTRIBUTE_UNUSED) { ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("ifaceIsVirtualFunction is not supported on non-linux " @@ -1326,8 +1326,9 @@ ifaceIsVirtualFunction(const char *ifname) } int -ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, - int *vf_index) +ifaceGetVirtualFunctionIndex(const char *pfname ATTRIBUTE_UNUSED, + const char *vfname ATTRIBUTE_UNUSED, + int *vf_index ATTRIBUTE_UNUSED) { ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("ifaceGetVirtualFunctionIndex is not supported on non-linux " @@ -1336,7 +1337,8 @@ ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, } int -ifaceGetPhysicalFunction(const char *ifname, char **pfname) +ifaceGetPhysicalFunction(const char *ifname ATTRIBUTE_UNUSED, + char **pfname ATTRIBUTE_UNUSED) { ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("ifaceGetPhysicalFunction is not supported on non-linux " diff --git a/src/util/pci.c b/src/util/pci.c index 7040e55..9873f33 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1685,6 +1685,8 @@ int pciDeviceIsAssignable(pciDevice *dev, return 1; } +#ifdef __linux__ + /* * returns 1 if equal and 0 if not */ @@ -1804,7 +1806,6 @@ out: return ret; } -#ifdef __linux__ /* * Returns Physical function given a virtual function */ @@ -2010,8 +2011,8 @@ out: } #else int -pciGetPhysicalFunction(const char *vf_sysfs_path, - struct pci_config_address **physical_function) +pciGetPhysicalFunction(const char *vf_sysfs_path ATTRIBUTE_UNUSED, + struct pci_config_address **physical_function ATTRIBUTE_UNUSED) { pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetPhysicalFunction is not " "supported on non-linux platforms")); @@ -2019,9 +2020,9 @@ pciGetPhysicalFunction(const char *vf_sysfs_path, } int -pciGetVirtualFunctions(const char *sysfs_path, - struct pci_config_address ***virtual_functions, - unsigned int *num_virtual_functions) +pciGetVirtualFunctions(const char *sysfs_path ATTRIBUTE_UNUSED, + struct pci_config_address ***virtual_functions ATTRIBUTE_UNUSED, + unsigned int *num_virtual_functions ATTRIBUTE_UNUSED) { pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetVirtualFunctions is not " "supported on non-linux platforms")); @@ -2029,7 +2030,7 @@ pciGetVirtualFunctions(const char *sysfs_path, } int -pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link) +pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link ATTRIBUTE_UNUSED) { pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceIsVirtualFunction is " "not supported on non-linux platforms")); @@ -2037,9 +2038,9 @@ pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link) } int -pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, - const char *vf_sysfs_device_link, - int *vf_index) +pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED, + const char *vf_sysfs_device_link ATTRIBUTE_UNUSED, + int *vf_index ATTRIBUTE_UNUSED) { pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetVirtualFunctionIndex is " "not supported on non-linux platforms")); @@ -2048,7 +2049,8 @@ pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, } int -pciDeviceNetName(char *device_link_sysfs_path, char **netname) +pciDeviceNetName(char *device_link_sysfs_path ATTRIBUTE_UNUSED, + char **netname ATTRIBUTE_UNUSED) { pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceNetName is not " "supported on non-linux platforms")); diff --git a/src/util/virfile.c b/src/util/virfile.c index 0edf058..1158998 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -44,7 +44,7 @@ int virFileClose(int *fdptr, bool preserve_errno) { - int saved_errno; + int saved_errno = 0; int rc = 0; if (*fdptr >= 0) { @@ -62,7 +62,7 @@ int virFileClose(int *fdptr, bool preserve_errno) int virFileFclose(FILE **file, bool preserve_errno) { - int saved_errno; + int saved_errno = 0; int rc = 0; if (*file) { diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 7dd3c51..1927051 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -213,6 +213,8 @@ int virPidFileReadPathIfAlive(const char *path, #ifdef __linux__ if (virFileLinkPointsTo(procpath, binpath) == 0) *pid = -1; +#else + (void)binpath; #endif VIR_FREE(procpath);

2 8

[libvirt] Bug - libvirt persists on having dnsmasq
by Zdenek Styblik 16 Aug '11

16 Aug '11

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, I've noticed libvirt is persisting to have dnsmasq present and use it. Version of libvirt in question is 0.9.4. There is no dnsmasq present on the Host during libvirt compilation, yet it seems to me ./configure wrongfully finds one: ~~~ SNIP ~~~ configure:46087: checking for dnsmasq configure:46119: result: dnsmasq ~~~ SNIP ~~~ shouldn't there be "result: no"? As far as I remember - no dnsmasq present during compilation, no usage of it later. However I might have missed something - change in behavior, requirements etc. This is possible. Still, if required "component" is missing, I say compilation resp. configure should fail. And this of course breaks things later on: ~~~ SNIP ~~~ virsh # net-start virtnet01 error: Failed to start network virtnet01 error: Cannot find 'dnsmasq' in path: No such file or directory ~~~ SNIP ~~~ and there is no DHCP defined for 'virtnet01' thus there is no reason to bring dnsmasq in. Neither can be new network created and started via virt-manager. Thanks, Zdenek - -- Zdenek Styblik email: stybla(a)turnovfree.net jabber: stybla(a)jabber.turnovfree.net -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5H2OwACgkQ8MreUbSH7ingtgCePYxU4zroZQo+3xrSxT7/hekN Qc8An3U/nDY/C8Z0oLQ76Za1SrR++0ON =TkKh -----END PGP SIGNATURE-----

4 10

[libvirt] [PATCH] Fix race condition in abort of stream
by Daniel P. Berrange 16 Aug '11

16 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> If a stream gets a server initiated abort, the client may still send an abort request before it receives the server side abort. This causes the server to send back another abort for the stream. Since the protocol defines that abort is the last thing to be sent, the client gets confused by this second abort from the server. If the stream is already shutdown, just drop any client requested abort, rather than sending back another message. This fixes the regression from previous versions. Tested as follows In one virsh session virsh # start foo virsh # console foo In other virsh session virsh # destroy foo The first virsh session should be able to continue issues commands without error. Prior to this patch it saw virsh # list error: Failed to list active domains error: An error occurred, but the cause is unknown virsh # list error: Failed to list active domains error: no call waiting for reply with prog 536903814 vers 1 serial 9 * src/rpc/virnetserverprogram.c: Drop abort requests for streams which no longer exist --- src/rpc/virnetserverprogram.c | 25 ++++++++----------------- 1 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 63a6b6d..ca80ae0 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -257,23 +257,14 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, * stream packets after we closed down a stream. Just drop & ignore * these. */ - if (msg->header.status == VIR_NET_CONTINUE) { - VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d", - msg->header.serial, msg->header.proc, msg->header.status); - /* Send a dummy reply to free up 'msg' & unblock client rx */ - memset(msg, 0, sizeof(*msg)); - msg->header.type = VIR_NET_REPLY; - if (virNetServerClientSendMessage(client, msg) < 0) { - ret = -1; - goto cleanup; - } - } else { - VIR_INFO("Unexpected stream control message serial=%d proc=%d status=%d", - msg->header.serial, msg->header.proc, msg->header.status); - virNetError(VIR_ERR_RPC, - _("Unexpected stream control message serial=%d proc=%d status=%d"), - msg->header.serial, msg->header.proc, msg->header.status); - goto error; + VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d", + msg->header.serial, msg->header.proc, msg->header.status); + /* Send a dummy reply to free up 'msg' & unblock client rx */ + memset(msg, 0, sizeof(*msg)); + msg->header.type = VIR_NET_REPLY; + if (virNetServerClientSendMessage(client, msg) < 0) { + ret = -1; + goto cleanup; } ret = 0; break; -- 1.7.6

2 1

[libvirt] [PATCH 0/4 v3] macvtap: Support for sending port profile message for a SRIOV VF to its PF
by Roopa Prabhu 16 Aug '11

16 Aug '11

This patch tries to fix getPhysFn in macvtap.c to get the physical function(PF) of the direct attach interface, if the interface is a SR-IOV VF. It moves some of the sriov pci device handling code from node_device_driver to src/util/pci.[ch]. This patch series implements the following 01/4 - pci: Move some pci sriov helper code out of node device driver to util/pci 02/4 - pci: Add helper functions for sriov devices 03/4 - interface: Add functions to get sriov PF/VF relationship of a net interface 04/4 - macvtap: Fix getPhysfn to get the PF of a direct attach network interface Changelog: ---------- v1: RFC patch introduced new functions to get PF/VF relationship of SRIOV net devices by comparing PF/VF sysfs device links. Also mentioned the existance of some related code in node_device_driver. The feedback was to move and use the node_device_driver code v2: Moves sriov get_physical_function and get_virtual_functions from node_device_driver to src/util/pci*. And reworks the rest of the patches around the new code. v3: Incorporated Stefan Berger's comments. Fixed indentation errors shown by 'make syntax-check'. Used pciReportError in src/util/pci Signed-off-by: Roopa Prabhu <roprabhu(a)cisco.com> Signed-off-by: Christian Benvenuti <benve(a)cisco.com> Signed-off-by: David Wang <dwang2(a)cisco.com>

2 10

[libvirt] [PATCH v3] daemon: Add early libvirtd start verbose errors.
by Peter Krempa 16 Aug '11

16 Aug '11

Early errors during start of libvirtd didn't have an error reporting mechanism and caused libvirtd to exit silently (only the return value indicated an error). Libvirt logging is initialized very early using enviroment variables and the internal error reporting API is used to report early errors. v2 changes: - print errors unconditionaly before logging starts - fix message to US spelling v2.5 changes: - initialize logging from enviroment - log all early errors using VIR_ERROR v3 changes: - move virSetLogFromEnv() after virInitialize() fixes: https://bugzilla.redhat.com/show_bug.cgi?id=728654 --- On 08/12/2011 12:09 PM, Daniel P. Berrange wrote: > Ok, not quiet that early. virInitialize() must be the first libvirt > call made since it initializes threading which is needed by logging. Uh, sorry. I didn't check for that. Now it should be in the correct place. > > Daniel Peter Krempa daemon/libvirtd.c | 39 +++++++++++++++++++++++++++++---------- 1 files changed, 29 insertions(+), 10 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 53f1002..c7382b0 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1293,6 +1293,9 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } + /* initialize early logging */ + virLogSetFromEnv(); + while (1) { int optidx = 0; int c; @@ -1328,14 +1331,18 @@ int main(int argc, char **argv) { case 'p': VIR_FREE(pid_file); - if (!(pid_file = strdup(optarg))) + if (!(pid_file = strdup(optarg))) { + VIR_ERROR(_("Can't allocate memory")); exit(EXIT_FAILURE); + } break; case 'f': VIR_FREE(remote_config_file); - if (!(remote_config_file = strdup(optarg))) + if (!(remote_config_file = strdup(optarg))) { + VIR_ERROR(_("Can't allocate memory")); exit(EXIT_FAILURE); + } break; case OPT_VERSION: @@ -1347,27 +1354,33 @@ int main(int argc, char **argv) { return 2; default: - fprintf (stderr, _("%s: internal error: unknown flag: %c\n"), - argv[0], c); + VIR_ERROR(_("%s: internal error: unknown flag: %c"), + argv[0], c); exit (EXIT_FAILURE); } } - if (!(config = daemonConfigNew(privileged))) + if (!(config = daemonConfigNew(privileged))) { + VIR_ERROR(_("Can't create initial configuration")); exit(EXIT_FAILURE); + } /* No explicit config, so try and find a default one */ if (remote_config_file == NULL) { implicit_conf = true; if (daemonConfigFilePath(privileged, - &remote_config_file) < 0) + &remote_config_file) < 0) { + VIR_ERROR(_("Can't determine config path")); exit(EXIT_FAILURE); + } } /* Read the config file if it exists*/ if (remote_config_file && - daemonConfigLoad(config, remote_config_file, implicit_conf) < 0) + daemonConfigLoad(config, remote_config_file, implicit_conf) < 0) { + VIR_ERROR(_("Can't load config file '%s'"), remote_config_file); exit(EXIT_FAILURE); + } if (config->host_uuid && virSetHostUUIDStr(config->host_uuid) < 0) { @@ -1375,19 +1388,25 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } - if (daemonSetupLogging(config, privileged, verbose, godaemon) < 0) + if (daemonSetupLogging(config, privileged, verbose, godaemon) < 0) { + VIR_ERROR(_("Can't initialize logging")); exit(EXIT_FAILURE); + } if (!pid_file && privileged && daemonPidFilePath(privileged, - &pid_file) < 0) + &pid_file) < 0) { + VIR_ERROR(_("Can't determine pid file path.")); exit(EXIT_FAILURE); + } if (daemonUnixSocketPaths(config, privileged, &sock_file, - &sock_file_ro) < 0) + &sock_file_ro) < 0) { + VIR_ERROR(_("Can't determine socket paths")); exit(EXIT_FAILURE); + } if (godaemon) { char ebuf[1024]; -- 1.7.6

2 1

[libvirt] Patch for verbose Disk transfer
by Tom Vijlbrief 16 Aug '11

16 Aug '11

A virsh command like: migrate --live --copy-storage-all Guest qemu+ssh://user@host/system --persistent --verbose shows Migration: [ 0 %] during the storage copy and does not start counting untill the ram transfer starts Patch for including the storage copy: diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..4ac8008 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1183,6 +1183,9 @@ cleanup: #define MIGRATION_TRANSFER_PREFIX "transferred ram: " #define MIGRATION_REMAINING_PREFIX "remaining ram: " #define MIGRATION_TOTAL_PREFIX "total ram: " +#define MIGRATION_DISK_TRANSFER_PREFIX "transferred disk: " +#define MIGRATION_DISK_REMAINING_PREFIX "remaining disk: " +#define MIGRATION_DISK_TOTAL_PREFIX "total disk: " int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, int *status, @@ -1246,6 +1249,7 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, goto cleanup; } *remaining *= 1024; + tmp = end; if (!(tmp = strstr(tmp, MIGRATION_TOTAL_PREFIX))) goto done; @@ -1257,7 +1261,53 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, goto cleanup; } *total *= 1024; + tmp = end; + + /* + * Check for Optional Disk Migration status + */ + + unsigned long long disk_transferred= 0; + unsigned long long disk_remaining= 0; + unsigned long long disk_total= 0; + + if (!(tmp = strstr(tmp, MIGRATION_DISK_TRANSFER_PREFIX))) + goto done; + tmp += strlen(MIGRATION_DISK_TRANSFER_PREFIX); + if (virStrToLong_ull(tmp, &end, 10, &disk_transferred) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse disk migration data transferred statistic %s"), tmp); + goto cleanup; + } + disk_transferred *= 1024; + (*transferred)+= disk_transferred; + tmp = end; + + if (!(tmp = strstr(tmp, MIGRATION_DISK_REMAINING_PREFIX))) + goto done; + tmp += strlen(MIGRATION_DISK_REMAINING_PREFIX); + + if (virStrToLong_ull(tmp, &end, 10, &disk_remaining) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse disk migration data remaining statistic %s"), tmp); + goto cleanup; + } + disk_remaining *= 1024; + (*remaining)+= disk_remaining; + tmp = end; + + if (!(tmp = strstr(tmp, MIGRATION_DISK_TOTAL_PREFIX))) + goto done; + tmp += strlen(MIGRATION_DISK_TOTAL_PREFIX); + + if (virStrToLong_ull(tmp, &end, 10, &disk_total) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse disk migration data total statistic %s"), tmp); + goto cleanup; + } + disk_total *= 1024; + (*total)+= disk_total; } }

2 1

[libvirt] maint: add missing symbols
by Stefan Berger 16 Aug '11

16 Aug '11

Add missing symbols to libvirt_private.syms. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/libvirt_private.syms | 8 ++++++++ 1 file changed, 8 insertions(+) Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -539,8 +539,11 @@ ifaceGetIndex; ifaceGetMacAddress; ifaceGetIPAddress; ifaceGetNthParent; +ifaceGetPhysicalFunction; +ifaceGetVirtualFunctionIndex; ifaceGetVlanID; ifaceIsUp; +ifaceIsVirtualFunction; ifaceLinkDel; ifaceMacvtapLinkAdd; ifaceMacvtapLinkDump; @@ -841,6 +844,7 @@ pciDettachDevice; pciDeviceFileIterate; pciDeviceGetManaged; pciDeviceIsAssignable; +pciDeviceIsVirtualFunction; pciDeviceListAdd; pciDeviceListCount; pciDeviceListDel; @@ -848,10 +852,14 @@ pciDeviceListFree; pciDeviceListGet; pciDeviceListNew; pciDeviceListSteal; +pciDeviceNetName; pciDeviceReAttachInit; pciDeviceSetManaged; pciFreeDevice; pciGetDevice; +pciGetPhysicalFunction; +pciGetVirtualFunctionIndex; +pciGetVirtualFunctions; pciReAttachDevice; pciResetDevice; pciWaitForDeviceCleanup;

2 1

Re: [libvirt] (no subject)
by jim_marshall＠msn.com 16 Aug '11

16 Aug '11

Visit it! Its cool and fantastic!. http://hosting.nborges.net/com.page.php?gusiteid=65o5

1 0

Re: [libvirt] [Spice-devel] libvirt multifunction problem
by Gerd Hoffmann 16 Aug '11

16 Aug '11

On 08/12/11 16:18, Christophe Fergeau wrote: > On Fri, Aug 12, 2011 at 04:53:04PM +0300, Alon Levy wrote: >> I don't know when libvirt started adding mf=on. > > For what it's worth, this was added in [ adding libvirt list to cc: ] > commit 9f8baf646ea44aa2de47cba5002af39a7ab1ac08 > Author: Wen Congyang<wency(a)cn.fujitsu.com> > Date: Mon May 23 14:42:21 2011 +0800 > > support multifunction PCI device > > If qemu supports multi function PCI device, the format of the PCI address passed > to qemu is "bus=pci.0,multifunction=on,addr=slot.function". > > If qemu does not support multi function PCI device, the format of the PCI address > passed to qemu is "bus=pci.0,addr=slot". Hmm, from a bief scan it looks like that patch adds multifunction=on unconditionally in case qemu supports that. That is wrong IMHO. libvirt should only do that in case the pci slot in question actually has multiple functions. There is another condition which the patch doesn't check for: you always must have a $slot.0 function, you can't have a multifunction device with -- say -- functions 1 and 2 being used. cheers, Gerd

1 0

[libvirt] [PATCH 0/2] about Virsh Command Reference
by Hu Tao 16 Aug '11

16 Aug '11

Hi, We now have libvirt advanced at 0.9.3, but there is almost no progress has been made on Virsh Command Reference. I'm now starting on updating Virsh Command Reference to get it synced with libvirt/virsh, but since I'm not a native-English speaker, there must be grammers, typos, etc. in the documents written by me, so any comments on these are appreciated. Besides, comments on other errors, such as example usages, are also welcome. In this series, docs for two cmmands are updated. I'll post more patches for other commands subsequently. Hu Tao (2): update documentation for command attach-disk update documentation for command detach-disk source/attach-disk.xml | 292 +++++++++++++++++++++++++++++++++++++++++++++++- source/detach-disk.xml | 233 +++++++++++++++++++++++++++++++++++++- 2 files changed, 517 insertions(+), 8 deletions(-) -- 1.7.3.1

4 6