Re: [libvirt] [PATCH v2] add --cache, --serial, --sharable and --pciaddress options(was Re: [PATCH 1/3] add --cache option for attach-disk)
On Tue, Jul 12, 2011 at 11:50:51AM -0600, Eric Blake wrote: On 07/12/2011 01:47 AM, Hu Tao wrote: +ignore_value(vshCommandOptString(cmd, cache, cache)); Not so nice. --cache '' will make vshCommandOptString return -1, because that usage is a virsh usage error and should be diagnosed as such up front, rather than accidentally passing cache='' through the XML to the libvirt API. I found that in the case of --cache '' vshCommandOptString returns 0, with cache(3rd parameter) unchanged. So can we safely ignore value or do I miss something? vshCommandOptString currently returns: 1 if the option was provided and non-empty, or provided and empty and VSH_OFLAG_EMPTY_OK 0 if the option was not provided, or provided but empty -1 if the option was not provided but VSH_OFLAG_REQ --cache isn't marked VSH_OFLAG_REQ, so we won't get -1. And we didn't pass VSH_OFLAG_EMPTY_OK, so an empty string returns 0 instead of 1, leaving the variable NULL the same as if --cache had not been provided. But your code would be the first instance of using ignore_value on vshCommandOptString. I'm starting to think that's a bug in the implementation, and that vshCommandOptString should instead return: 1 if the option was provided and non-empty, or provided and empty and VSH_OFLAG_EMPTY_OK 0 if the option was not provided -1 if the option was not provided but VSH_OFLAG_REQ, or provided and empty and not VSH_OFLAG_EMPTY_OK Sounds good. in which case, it _does_ become important to check for errors. @@ -10209,6 +10254,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) goto cleanup; } +ignore_value(vshCommandOptString(cmd, cache, cache)); +ignore_value(vshCommandOptString(cmd, serial, serial)); +ignore_value(vshCommandOptString(cmd, pciaddress, strpciaddr)); At any rate, we already have lots of existing code that looks like: if (vshCommandOptString(cmd, cache, cache) 0 || vshCommandOptString(cmd, serial, serial) 0 || vshCommandOptString(cmd, pciaddress, strpciaddr) 0) { vshError(ctl, %s, _(missing argument)); goto out; } Yes, the code should looks like this. I didn't noticed VSH_OFLAG_REQ(and friends). Will update in v3. -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] vcpubandwidth: introduce two new libvirt APIs
At 07/07/2011 10:32 AM, Taku Izumi Write: So why introduce VCPU level apis? Adam Litke said IBM's performance team nead to control cpu bandwidth for each vcpu. Right, but we do not export that as a User API, that was my suggestion. We can internally control each vcpu's bandwidth, i.e. divide equally. Hmm, I heard that some server could run CPUs at different speed. May be this patch can simulate this behavior. That happens on my laptop as well, depending on the machine load CPU frequency is changed but it is done transparently. I means explicitly CPU speed configuring. ;) I am not sure if we are trying to simulate that here. So why not leave the flexible interface here, and let users make the decision? In my mind, the flexibility is not always a good thing. It is nothing but troublesome for the person who doesn't like detailed setting. I don't know how many people want this flexibility. I think we should implement the flexibility. If we do not implement, and we want it later, we can not reuse these codes(add new element, and reimplement). I'd prefer the flexibility. IBM's performance team has tested this patch, and they said this patch produced the expected and desired performance characteristics. I'm for Nikunj's desgin. Do you have any concerns? Hi, izumi-san, Do you still object this implement? -- Best regards, Taku Izumi izumi.t...@jp.fujitsu.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/19] qemu: Consolidate qemuMigrationPrepare{Direct, Tunnel}
On Mon, Jul 11, 2011 at 21:28:30 +0100, Daniel P. Berrange wrote: On Fri, Jul 08, 2011 at 01:34:13AM +0200, Jiri Denemark wrote: Most of the code in these two functions is supposed to be identical but currently it isn't (which is natural since the code is duplicated). Let's move common parts of these functions into qemuMigrationPrepareAny. Were there any actual bugs fixed in this consolidation, or were the differences in the code just cosmetic ? If so it'd be good to mention the bugs fixed in the commit message Yes, I added the following to the commit message: This also fixes qemuMigrationPrepareTunnel which didn't store received lockState in the domain object. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/19] Rollback migration when libvirtd restarts
On Fri, Jul 08, 2011 at 01:34:05 +0200, Jiri Denemark wrote: This series is also available at https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery The series does several things: - persists current job and its phase in status xml - allows safe monitor commands to be run during migration/save/dump jobs - implements recovery when libvirtd is restarted while a job is active - consolidates some code and fixes bugs I found when working in the area The series is not perfect and still needs some corner cases to be fixed but I think it's better to send the series for review now and add small additional fixes in the next version(s) instead of waiting for it to be perfect. OK, I pushed the following patches (01-08/19 and 13/19) which were already acked. When doing so, I also updated documentation in src/qemu/THREADS.txt as part of the qemu: Allow all query commands to be run during long jobs patch. The diff to THREADS.txt is attached. qemu: Separate job related data into a new object qemu: Consolidate BeginJob{,WithDriver} into a single method qemu: Consolidate {Enter,Exit}Monitor{,WithDriver} qemu: Allow all query commands to be run during long jobs qemu: Save job type in domain status XML qemu: Recover from interrupted jobs qemu: Add support for job phase qemu: Consolidate qemuMigrationPrepare{Direct,Tunnel} qemu: Fix monitor unlocking in some error paths Jirka diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt index 1e0b5ab..3a27a85 100644 --- a/src/qemu/THREADS.txt +++ b/src/qemu/THREADS.txt @@ -49,17 +49,39 @@ There are a number of locks on various objects - * qemuMonitorPrivatePtr: Job condition + * qemuMonitorPrivatePtr: Job conditions Since virDomainObjPtr lock must not be held during sleeps, the job -condition provides additional protection for code making updates. +conditions provide additional protection for code making updates. + +Qemu driver uses two kinds of job conditions: asynchronous and +normal. + +Asynchronous job condition is used for long running jobs (such as +migration) that consist of several monitor commands and it is +desirable to allow calling a limited set of other monitor commands +while such job is running. This allows clients to, e.g., query +statistical data, cancel the job, or change parameters of the job. + +Normal job condition is used by all other jobs to get exclusive +access to the monitor and also by every monitor command issued by an +asynchronous job. When acquiring normal job condition, the job must +specify what kind of action it is about to take and this is checked +against the allowed set of jobs in case an asynchronous job is +running. If the job is incompatible with current asynchronous job, +it needs to wait until the asynchronous job ends and try to acquire +the job again. Immediately after acquiring the virDomainObjPtr lock, any method -which intends to update state must acquire the job condition. The -virDomainObjPtr lock is released while blocking on this condition -variable. Once the job condition is acquired, a method can safely -release the virDomainObjPtr lock whenever it hits a piece of code -which may sleep/wait, and re-acquire it after the sleep/wait. +which intends to update state must acquire either asynchronous or +normal job condition. The virDomainObjPtr lock is released while +blocking on these condition variables. Once the job condition is +acquired, a method can safely release the virDomainObjPtr lock +whenever it hits a piece of code which may sleep/wait, and +re-acquire it after the sleep/wait. Whenever an asynchronous job +wants to talk to the monitor, it needs to acquire nested job (a +special kind of normla job) to obtain exclusive access to the +monitor. Since the virDomainObjPtr lock was dropped while waiting for the job condition, it is possible that the domain is no longer active @@ -111,31 +133,74 @@ To lock the virDomainObjPtr -To acquire the job mutex +To acquire the normal job condition qemuDomainObjBeginJob() (if driver is unlocked) - Increments ref count on virDomainObjPtr -- Wait qemuDomainObjPrivate condition 'jobActive != 0' using - virDomainObjPtr mutex -- Sets jobActive to 1 +- Waits until the job is compatible with current async job or no + async job is running +- Waits job.cond condition 'job.active != 0' using virDomainObjPtr + mutex +- Rechecks if the job is still compatible and repeats waiting if it + isn't +- Sets job.active to the job type qemuDomainObjBeginJobWithDriver() (if driver needs to be locked) -- Unlocks driver - Increments ref count on virDomainObjPtr -- Wait qemuDomainObjPrivate condition 'jobActive != 0' using - virDomainObjPtr mutex -- Sets jobActive to 1 +- Unlocks driver +
Re: [libvirt] [PATCH v2 0/7] Add support for setting QoS
On Tue, Jul 12, 2011 at 12:57 PM, Michal Privoznik mpriv...@redhat.com wrote: This patch series add support for setting traffic shaping and policing on both domain's interface and network's virtual bridge. Basically, this is done via 'tc' from iproute2 package. For shaping is HTB used, for policing we need u32 match selector. Both should be available in RHEL-6 kernel. Nice, this is a good feature. I'll keep an eye on this! Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add domain events support to UML driver
On Mon, Jul 04, 2011 at 13:32:23 +0100, Daniel P. Berrange wrote: * src/uml_conf.h: Add queue for dispatch of domain events * src/uml_driver.c: Trigger domain events upon important lifecycle transitions Does it mean someone is actually using UML? :-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 91591f1..d0736df 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c ... @@ -2208,6 +2282,114 @@ cleanup: } +static int +umlDomainEventRegister(virConnectPtr conn, + virConnectDomainEventCallback callback, + void *opaque, + virFreeCallback freecb) +{ +struct uml_driver *driver = conn-privateData; +int ret; + +umlDriverLock(driver); +ret = virDomainEventCallbackListAdd(conn, +driver-domainEventState-callbacks, +callback, opaque, freecb); +umlDriverUnlock(driver); + +return ret; +} +static int +umlDomainEventDeregister(virConnectPtr conn, + virConnectDomainEventCallback callback) +{ +struct uml_driver *driver = conn-privateData; +int ret; + +umlDriverLock(driver); +ret = virDomainEventStateDeregister(conn, +driver-domainEventState, +callback); +umlDriverUnlock(driver); + +return ret; +} +static int +umlDomainEventRegisterAny(virConnectPtr conn, ... Nit: an empty line is missing between these two functions. ACK with that fixed. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix missing prompt message for 'snapshot-delete' command
Make the command 'virsh snapshot-delete' has the appropriate prompt message when executing sucessful or failed. --- tools/virsh.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index cd17f42..b7cea58 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11534,8 +11534,12 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) if (snapshot == NULL) goto cleanup; -if (virDomainSnapshotDelete(snapshot, flags) 0) +if (virDomainSnapshotDelete(snapshot, flags) == 0) { +vshPrint(ctl, _(Domain snapshot %s deleted\n), name); +} else { +vshError(ctl, _(Failed to delete snapshot %s), name); goto cleanup; +} ret = true; -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] vcpubandwidth: introduce two new libvirt APIs
On Wed, 13 Jul 2011 14:26:23 +0800, Wen Congyang we...@cn.fujitsu.com wrote: At 07/07/2011 10:32 AM, Taku Izumi Write: So why introduce VCPU level apis? Adam Litke said IBM's performance team nead to control cpu bandwidth for each vcpu. Right, but we do not export that as a User API, that was my suggestion. We can internally control each vcpu's bandwidth, i.e. divide equally. Hmm, I heard that some server could run CPUs at different speed. May be this patch can simulate this behavior. That happens on my laptop as well, depending on the machine load CPU frequency is changed but it is done transparently. I means explicitly CPU speed configuring. ;) I am not sure if we are trying to simulate that here. So why not leave the flexible interface here, and let users make the decision? In my mind, the flexibility is not always a good thing. It is nothing but troublesome for the person who doesn't like detailed setting. I don't know how many people want this flexibility. I think we should implement the flexibility. If we do not implement, and we want it later, we can not reuse these codes(add new element, and reimplement). IMHO, at present we can use the current SetSchedulerParameters API and whenever we need flexibility an API as suggested in this series could be added. Thanks Nikunj -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] vcpubandwidth: introduce two new libvirt APIs
At 07/13/2011 04:50 PM, Nikunj A. Dadhania Write: On Wed, 13 Jul 2011 14:26:23 +0800, Wen Congyang we...@cn.fujitsu.com wrote: At 07/07/2011 10:32 AM, Taku Izumi Write: So why introduce VCPU level apis? Adam Litke said IBM's performance team nead to control cpu bandwidth for each vcpu. Right, but we do not export that as a User API, that was my suggestion. We can internally control each vcpu's bandwidth, i.e. divide equally. Hmm, I heard that some server could run CPUs at different speed. May be this patch can simulate this behavior. That happens on my laptop as well, depending on the machine load CPU frequency is changed but it is done transparently. I means explicitly CPU speed configuring. ;) I am not sure if we are trying to simulate that here. So why not leave the flexible interface here, and let users make the decision? In my mind, the flexibility is not always a good thing. It is nothing but troublesome for the person who doesn't like detailed setting. I don't know how many people want this flexibility. I think we should implement the flexibility. If we do not implement, and we want it later, we can not reuse these codes(add new element, and reimplement). IMHO, at present we can use the current SetSchedulerParameters API and whenever we need flexibility an API as suggested in this series could be If we need flexibilty, not only an API shoule be added. We should add new element in the XML config file. It means that libvirt should support inflexibility and flexibilty. It is very bad. If we want to support flexibility later, it is better to support it now. Thanks. Wen Congyang added. Thanks Nikunj -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Matahari] [libvirt-qpid PATCH 2/3] Convert class names to lower case
On Wed, Jul 13, 2011 at 2:29 AM, Zane Bitter zbit...@redhat.com wrote: This schema change is necessary to ensure that classes keep the same names after changing from the QMF to the QMFv2 API. Odd. But if you say so, ACK :-) --- src/DomainWrap.cpp | 2 +- src/NodeWrap.cpp | 2 +- src/PoolWrap.cpp | 2 +- src/VolumeWrap.cpp | 2 +- src/libvirt-schema.xml | 8 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index eab6bbc..a72d970 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -12,7 +12,7 @@ DomainWrap::DomainWrap( virDomainPtr domain_ptr, virConnectPtr conn): PackageOwnerNodeWrap::PackageDefinition(parent), - ManagedObject(package().data_Domain), + ManagedObject(package().data_domain), _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN]; diff --git a/src/NodeWrap.cpp b/src/NodeWrap.cpp index d65db11..40c4339 100644 --- a/src/NodeWrap.cpp +++ b/src/NodeWrap.cpp @@ -16,7 +16,7 @@ NodeWrap::NodeWrap(qmf::AgentSession agent_session, PackageDefinition package): - ManagedObject(package.data_Node), + ManagedObject(package.data_node), _session(agent_session), _package(package) { diff --git a/src/PoolWrap.cpp b/src/PoolWrap.cpp index a5992f2..dee2597 100644 --- a/src/PoolWrap.cpp +++ b/src/PoolWrap.cpp @@ -13,7 +13,7 @@ PoolWrap::PoolWrap(NodeWrap *parent, virStoragePoolPtr pool_ptr, virConnectPtr connect): PackageOwnerNodeWrap::PackageDefinition(parent), - ManagedObject(package().data_Pool), + ManagedObject(package().data_pool), _pool_ptr(pool_ptr), _conn(connect) { int ret; diff --git a/src/VolumeWrap.cpp b/src/VolumeWrap.cpp index fcda96e..59b21d8 100644 --- a/src/VolumeWrap.cpp +++ b/src/VolumeWrap.cpp @@ -16,7 +16,7 @@ VolumeWrap::VolumeWrap(PoolWrap *parent, virStorageVolPtr volume_ptr, virConnectPtr connect): PackageOwnerPoolWrap::PackageDefinition(parent), - ManagedObject(package().data_Volume), + ManagedObject(package().data_volume), _volume_ptr(volume_ptr), _conn(connect), _lvm_name(), _has_lvm_child(false), _wrap_parent(parent) diff --git a/src/libvirt-schema.xml b/src/libvirt-schema.xml index a1b88e1..6fbca76 100644 --- a/src/libvirt-schema.xml +++ b/src/libvirt-schema.xml @@ -7,7 +7,7 @@ !-- In libvirt this is really the 'Connect' class, I may end up changing it.. -- - class name=Node + class name=node property name=hostname type=sstr access=RC desc=Host name index=y/ property name=uri type=sstr access=RC desc=URI of libvirt/ property name=libvirtVersion type=sstr access=RC desc=Version of libvirt on the managed node/ @@ -54,7 +54,7 @@ /method /class - class name=Domain + class name=domain property name=uuid access=RC type=sstr desc=Domain UUID index=y/ property name=name access=RC type=sstr desc=Domain name index=y/ property name=id type=int64 desc=Hypervisor Domain id, -1 if not running./ @@ -109,7 +109,7 @@ /method /class - class name=Pool + class name=pool property name=uuid access=RC type=sstr desc=Pool UUID index=y/ property name=name access=RC type=sstr desc=Pool name index=y/ property name=parentVolume access=RC type=sstr desc=If this pool is an LVM pool, this will contain the parent volume path./ @@ -154,7 +154,7 @@ /class - class name=Volume + class name=volume property name=key type=sstr access=RC desc=Storage volume key index=y/ property name=path type=sstr access=RC desc=Storage volume path index=y/ property name=name type=sstr access=RC desc=Storage volume name index=y/ ___ Matahari mailing list matah...@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/matahari -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Matahari] [libvirt-qpid PATCH 1/3] Convert to QMFv2 APIs
On Wed, Jul 13, 2011 at 2:46 AM, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Jul 12, 2011 at 06:28:46PM +0200, Zane Bitter wrote: diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index 47876de..eab6bbc 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -2,266 +2,310 @@ #include NodeWrap.h #include DomainWrap.h #include Error.h +#include Exception.h -#include ArgsDomainMigrate.h -#include ArgsDomainRestore.h -#include ArgsDomainSave.h -#include ArgsDomainGetXMLDesc.h +#include cstdio -namespace _qmf = qmf::com::redhat::libvirt; -DomainWrap::DomainWrap(ManagementAgent *agent, NodeWrap *parent, - virDomainPtr _domain_ptr, virConnectPtr _conn) : - domain_ptr(_domain_ptr), conn(_conn) +DomainWrap::DomainWrap( + NodeWrap *parent, + virDomainPtr domain_ptr, + virConnectPtr conn): + PackageOwnerNodeWrap::PackageDefinition(parent), + ManagedObject(package().data_Domain), + _domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN]; - domain = NULL; - - if (virDomainGetUUIDString(domain_ptr, dom_uuid) 0) { - REPORT_ERR(conn, DomainWrap: Unable to get UUID string of domain.); + if (virDomainGetUUIDString(_domain_ptr, dom_uuid) 0) { + REPORT_ERR(_conn, DomainWrap: Unable to get UUID string of domain.); throw 1; I've noticed that most (all?) objects are being passed in a virConnectPtr instance too. This should be redundant for two reasons - If you have a virDomainPtr, you can call virDomainGetConnect() to get back the associated virConnectPtr (likewise for other libvirt objects) - We should kill the 'virConnectPtr' parameter from the REPORT_ERR macro/functions, and use 'virGetLastError' exclusively. This function is thread-safe, unlike the current virConnGetLastError being used. Since this problem was pre-existing, I'll leave it upto you whether you want to incorporate such a change into this patch or do it as a later followup. Personally I'd prefer it be in a follow-up patch. void DomainWrap::update() { virDomainInfo info; int ret; - ret = virDomainGetInfo(domain_ptr, info); + ret = virDomainGetInfo(_domain_ptr, info); if (ret 0) { - REPORT_ERR(conn, Domain get info failed.); + REPORT_ERR(_conn, Domain get info failed.); /* Next domainSync() will take care of this in the node wrapper if the domain is * indeed dead. */ return; } else { + const char *state = NULL; switch (info.state) { - case VIR_DOMAIN_NOSTATE: Add a 'default:' clause here - domain-set_state(nostate); + state = nostate; break; case VIR_DOMAIN_RUNNING: - domain-set_state(running); + state = running; break; case VIR_DOMAIN_BLOCKED: - domain-set_state(blocked); + state = blocked; break; case VIR_DOMAIN_PAUSED: - domain-set_state(paused); + state = paused; break; case VIR_DOMAIN_SHUTDOWN: - domain-set_state(shutdown); + state = shutdown; break; case VIR_DOMAIN_SHUTOFF: - domain-set_state(shutoff); + state = shutoff; break; case VIR_DOMAIN_CRASHED: - domain-set_state(crashed); + state = crashed; break; } - domain-set_numVcpus(info.nrVirtCpu); - domain-set_maximumMemory(info.maxMem); - domain-set_memory(info.memory); - domain-set_cpuTime(info.cpuTime); + if (state) { + _data.setProperty(state, state); + } We shouldn't skip out 'NOSTATE' here, juust make this unconditional + _data.setProperty(numVcpus, info.nrVirtCpu); + _data.setProperty(maximumMemory, info.maxMem); + _data.setProperty(memory, (uint64_t)info.memory); + _data.setProperty(cpuTime, (uint64_t)info.cpuTime); } +bool +DomainWrap::migrate(qmf::AgentSession session, qmf::AgentEvent event) +{ + virConnectPtr dest_conn; + virDomainPtr rem_dom; + qpid::types::Variant::Map args(event.getArguments()); + int ret; + + // This is actually quite broken. Most setups won't allow unauthorized + // connection from one node to another directly like this. + dest_conn = virConnectOpen(args[destinationUri].asString().c_str()); Since the agent was originally written, we now have a new migration API which avoids the need for a destination virConnectPtr object. Instead you can pass the 'destinationUri' directly to virDomainMigrateToURI, and set the PEER2PEER flag. NB, this still presumes that
Re: [libvirt] [Matahari] [libvirt-qpid PATCH 3/3] Make libvirt-qpid a matahari agent
Looks good to me. Ack On Wed, Jul 13, 2011 at 2:29 AM, Zane Bitter zbit...@redhat.com wrote: --- AUTHORS | 2 configure.ac | 1 libvirt-qpid.spec | 2 src/LibvirtAgent.cpp | 97 src/LibvirtAgent.h | 36 + src/Makefile.am | 7 +- src/NodeWrap.cpp | 203 +- src/NodeWrap.h | 22 + 8 files changed, 169 insertions(+), 201 deletions(-) create mode 100644 src/LibvirtAgent.cpp create mode 100644 src/LibvirtAgent.h diff --git a/AUTHORS b/AUTHORS index 367ea49..8af5aba 100644 --- a/AUTHORS +++ b/AUTHORS @@ -9,6 +9,6 @@ Patches have also been contributed by: Ted Ross tr...@redhat.com -Conversion to QMFv2 API by: +Conversion to matahari agent and QMFv2 API by: Zane Bitter zbit...@redhat.com diff --git a/configure.ac b/configure.ac index 3896d9d..03bbb80 100644 --- a/configure.ac +++ b/configure.ac @@ -21,6 +21,7 @@ dnl Required minimum versions of all libs we depend on LIBVIRT_REQUIRED=4.4.0 PKG_CHECK_MODULES(XML, libxml-2.0) +PKG_CHECK_MODULES([DEPS], [glib-2.0]) AC_OUTPUT(Makefile src/Makefile doc/Makefile) diff --git a/libvirt-qpid.spec b/libvirt-qpid.spec index d64552d..c8a88fd 100644 --- a/libvirt-qpid.spec +++ b/libvirt-qpid.spec @@ -10,6 +10,7 @@ Requires: libxml2 = 2.7.1 Requires: qmf = 0.5.790661 Requires: qpid-cpp-client = 0.10 Requires: libvirt = 0.4.4 +Requires: matahari-lib = 0.4.1 Requires(post): /sbin/chkconfig Requires(preun): /sbin/chkconfig Requires(preun): initscripts @@ -17,6 +18,7 @@ BuildRequires: qpid-cpp-client-devel = 0.10 BuildRequires: libxml2-devel = 2.7.1 BuildRequires: libvirt-devel = 0.5.0 BuildRequires: qmf-devel = 0.5.790661 +BuildRequires: matahari-devel = 0.4.1 Url: http://libvirt.org/qpid %description diff --git a/src/LibvirtAgent.cpp b/src/LibvirtAgent.cpp new file mode 100644 index 000..deb77ca --- /dev/null +++ b/src/LibvirtAgent.cpp @@ -0,0 +1,97 @@ +#include LibvirtAgent.h +#include NodeWrap.h +#include Exception.h + +#include syslog.h + + +#define POLL_TIME_s 3 + + +static gboolean handleTimer(gpointer user_data) +{ + LibvirtAgent *agent = (LibvirtAgent *)user_data; + if (agent) { + agent-updateData(); + return TRUE; + } + + return FALSE; +} + +int +LibvirtAgent::setup(qmf::AgentSession session) +{ + _package.configure(session); + initErrorSchema(session); + + _node = new NodeWrap(this); + + _timer = g_timeout_add_seconds(POLL_TIME_s, handleTimer, this); + + return 0; +} + +LibvirtAgent::~LibvirtAgent() +{ + if (_timer) { + g_source_remove(_timer); + } + delete _node; +} + +void +LibvirtAgent::addData(qmf::Data data) +{ + _agent_session.addData(data); +} + +void +LibvirtAgent::delData(qmf::Data data) +{ + _agent_session.delData(data.getAddr()); +} + +gboolean +LibvirtAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event, + gpointer user_data) +{ + if (event.getType() != qmf::AGENT_METHOD) { + return TRUE; + } + + bool handled = _node-handleMethod(session, event); + if (!handled) { + raiseException(session, event, + ERROR_UNKNOWN_OBJECT, STATUS_UNKNOWN_OBJECT); + } + + return TRUE; +} + +void +LibvirtAgent::updateData(void) +{ + if (_node) { + _node-poll(); + } +} + +int +main(int argc, char **argv) +{ + LibvirtAgent agent; + + int rc = agent.init(argc, argv, libvirt); + + openlog(libvirt-qpid, 0, LOG_DAEMON); + + // This prevents us from dying if libvirt disconnects. + signal(SIGPIPE, SIG_IGN); + + if (rc == 0) { + agent.run(); + } + return rc; +} + diff --git a/src/LibvirtAgent.h b/src/LibvirtAgent.h new file mode 100644 index 000..3b2a940 --- /dev/null +++ b/src/LibvirtAgent.h @@ -0,0 +1,36 @@ +#ifndef LIBVIRT_AGENT_H +#define LIBVIRT_AGENT_H + +#include matahari/mh_agent.h +#include QmfPackage.h +#include ManagedObject.h + + +class NodeWrap; + + +class LibvirtAgent: + public MatahariAgent, + public PackageOwnerqmf::com::redhat::libvirt::PackageDefinition +{ +public: + ~LibvirtAgent(); + + int setup(qmf::AgentSession session); + gboolean invoke(qmf::AgentSession session, qmf::AgentEvent event, + gpointer user_data); + + PackageDefinition package(void) { return _package; } + void addData(qmf::Data data); + void delData(qmf::Data data); + + void updateData(void); + +private: + PackageDefinition _package; + NodeWrap *_node; + int _timer; +}; + +#endif + diff --git a/src/Makefile.am b/src/Makefile.am index 4bb27b2..40d9371 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce
Re: [libvirt] [PATCHv2 01/27] maint: exclude more files from syntax check
2011/7/8 Eric Blake ebl...@redhat.com: * cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt docs/api_extension/*.patch. (exclude_file_name_regexp--sc_prohibit_always_true_header_tests) (exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF) (exclude_file_name_regexp--sc_prohibit_fork_wrappers) (exclude_file_name_regexp--sc_trailing_blank): Simplify. (exclude_file_name_regexp--sc_prohibit_gettext_noop): Delete. (exclude_file_name_regexp--sc_prohibit_close) (exclude_file_name_regexp--sc_prohibit_nonreentrant) (exclude_file_name_regexp--sc_prohibit_sprintf): Tighten. --- v2: new patch cfg.mk | 19 +-- 1 files changed, 9 insertions(+), 10 deletions(-) Looks good and make syntax-check still passes, ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't overwrite errors by closefd in error paths
When qemuMonitorCloseFileHandle is called in error path, we need to preserve the original error since a possible further error when running closefd monitor command is not very useful to users. --- src/qemu/qemu_monitor.c | 34 +- src/qemu/qemu_monitor.h |3 ++- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e593642..4c6c66f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1552,7 +1552,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon, ret = qemuMonitorTextMigrate(mon, flags, fd:migrate); if (ret 0) { -if (qemuMonitorCloseFileHandle(mon, migrate) 0) +if (qemuMonitorCloseFileHandle(mon, migrate, true) 0) VIR_WARN(failed to close migration handle); } @@ -1962,22 +1962,34 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon, int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, - const char *fdname) + const char *fdname, + bool preserveError) { -int ret; +int ret = -1; +virErrorPtr error = NULL; + VIR_DEBUG(mon=%p fdname=%s, mon, fdname); +if (preserveError) +error = virSaveLastError(); + if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, %s, _(monitor must not be NULL)); -return -1; +goto cleanup; } if (mon-json) ret = qemuMonitorJSONCloseFileHandle(mon, fdname); else ret = qemuMonitorTextCloseFileHandle(mon, fdname); + +cleanup: +if (error) { +virSetError(error); +virFreeError(error); +} return ret; } @@ -2014,9 +2026,11 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, cleanup: if (ret 0) { -if (tapfd = 0 qemuMonitorCloseFileHandle(mon, tapfd_name) 0) +if (tapfd = 0 +qemuMonitorCloseFileHandle(mon, tapfd_name, true) 0) VIR_WARN(failed to close device handle '%s', tapfd_name); -if (vhostfd = 0 qemuMonitorCloseFileHandle(mon, vhostfd_name) 0) +if (vhostfd = 0 +qemuMonitorCloseFileHandle(mon, vhostfd_name, true) 0) VIR_WARN(failed to close device handle '%s', vhostfd_name); } @@ -2078,9 +2092,11 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, cleanup: if (ret 0) { -if (tapfd = 0 qemuMonitorCloseFileHandle(mon, tapfd_name) 0) +if (tapfd = 0 +qemuMonitorCloseFileHandle(mon, tapfd_name, true) 0) VIR_WARN(failed to close device handle '%s', tapfd_name); -if (vhostfd = 0 qemuMonitorCloseFileHandle(mon, vhostfd_name) 0) +if (vhostfd = 0 +qemuMonitorCloseFileHandle(mon, vhostfd_name, true) 0) VIR_WARN(failed to close device handle '%s', vhostfd_name); } @@ -2258,7 +2274,7 @@ int qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon, ret = qemuMonitorTextAddDevice(mon, devicestr); if (ret 0 fd = 0) { -if (qemuMonitorCloseFileHandle(mon, fdname) 0) +if (qemuMonitorCloseFileHandle(mon, fdname, true) 0) VIR_WARN(failed to close device handle '%s', fdname); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 893f3e9..71ee932 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -363,7 +363,8 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon, int fd); int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, - const char *fdname); + const char *fdname, + bool preserveError); /* XXX do we really want to hardcode 'netstr' as the -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Matahari] [libvirt-qpid PATCH 1/3] Convert to QMFv2 APIs
Hi Daniel, Thanks for the very helpful (and prompt!) feedback. Responses inline. cheers, Zane. On 13/07/11 08:49, Andrew Beekhof wrote: On Wed, Jul 13, 2011 at 2:46 AM, Daniel P. Berrangeberra...@redhat.com wrote: On Tue, Jul 12, 2011 at 06:28:46PM +0200, Zane Bitter wrote: diff --git a/src/DomainWrap.cpp b/src/DomainWrap.cpp index 47876de..eab6bbc 100644 --- a/src/DomainWrap.cpp +++ b/src/DomainWrap.cpp @@ -2,266 +2,310 @@ #include NodeWrap.h #include DomainWrap.h #include Error.h +#include Exception.h -#include ArgsDomainMigrate.h -#include ArgsDomainRestore.h -#include ArgsDomainSave.h -#include ArgsDomainGetXMLDesc.h +#includecstdio -namespace _qmf = qmf::com::redhat::libvirt; -DomainWrap::DomainWrap(ManagementAgent *agent, NodeWrap *parent, - virDomainPtr _domain_ptr, virConnectPtr _conn) : - domain_ptr(_domain_ptr), conn(_conn) +DomainWrap::DomainWrap( +NodeWrap *parent, +virDomainPtr domain_ptr, +virConnectPtr conn): +PackageOwnerNodeWrap::PackageDefinition(parent), +ManagedObject(package().data_Domain), +_domain_ptr(domain_ptr), _conn(conn) { char dom_uuid[VIR_UUID_STRING_BUFLEN]; -domain = NULL; - -if (virDomainGetUUIDString(domain_ptr, dom_uuid) 0) { -REPORT_ERR(conn, DomainWrap: Unable to get UUID string of domain.); +if (virDomainGetUUIDString(_domain_ptr, dom_uuid) 0) { +REPORT_ERR(_conn, DomainWrap: Unable to get UUID string of domain.); throw 1; I've noticed that most (all?) objects are being passed in a virConnectPtr instance too. This should be redundant for two reasons - If you have a virDomainPtr, you can call virDomainGetConnect() to get back the associated virConnectPtr (likewise for other libvirt objects) - We should kill the 'virConnectPtr' parameter from the REPORT_ERR macro/functions, and use 'virGetLastError' exclusively. This function is thread-safe, unlike the current virConnGetLastError being used. Since this problem was pre-existing, I'll leave it upto you whether you want to incorporate such a change into this patch or do it as a later followup. Personally I'd prefer it be in a follow-up patch. Agree, this is a good candidate for a follow-up. void DomainWrap::update() { virDomainInfo info; int ret; -ret = virDomainGetInfo(domain_ptr,info); +ret = virDomainGetInfo(_domain_ptr,info); if (ret 0) { -REPORT_ERR(conn, Domain get info failed.); +REPORT_ERR(_conn, Domain get info failed.); /* Next domainSync() will take care of this in the node wrapper if the domain is * indeed dead. */ return; } else { +const char *state = NULL; switch (info.state) { - case VIR_DOMAIN_NOSTATE: Add a 'default:' clause here -domain-set_state(nostate); +state = nostate; break; case VIR_DOMAIN_RUNNING: -domain-set_state(running); +state = running; break; case VIR_DOMAIN_BLOCKED: -domain-set_state(blocked); +state = blocked; break; case VIR_DOMAIN_PAUSED: -domain-set_state(paused); +state = paused; break; case VIR_DOMAIN_SHUTDOWN: -domain-set_state(shutdown); +state = shutdown; break; case VIR_DOMAIN_SHUTOFF: -domain-set_state(shutoff); +state = shutoff; break; case VIR_DOMAIN_CRASHED: -domain-set_state(crashed); +state = crashed; break; } -domain-set_numVcpus(info.nrVirtCpu); -domain-set_maximumMemory(info.maxMem); -domain-set_memory(info.memory); -domain-set_cpuTime(info.cpuTime); +if (state) { +_data.setProperty(state, state); +} We shouldn't skip out 'NOSTATE' here, juust make this unconditional +_data.setProperty(numVcpus, info.nrVirtCpu); +_data.setProperty(maximumMemory, info.maxMem); +_data.setProperty(memory, (uint64_t)info.memory); +_data.setProperty(cpuTime, (uint64_t)info.cpuTime); } Fixed. Thanks! There is a similar issue with the state of a Pool (as written it will segfault if a state other than inactive/building/running/degraded is returned). What would be an appropriate default there? Just set it to unknown? +bool +DomainWrap::migrate(qmf::AgentSession session, qmf::AgentEvent event) +{ +virConnectPtr dest_conn; +virDomainPtr rem_dom; +qpid::types::Variant::Map args(event.getArguments()); +int ret; + +// This is actually quite broken. Most setups won't allow unauthorized +// connection from one node to another directly like
[libvirt] [PATCH 0/8] Introduce new API virDomainUndefineFlags
Hi, Per discussion on https://www.redhat.com/archives/libvir-list/2011-July/msg00556.html, This patch series introduce new API virDomainUndefineFlags, which only support flag VIR_DOMAIN_UNDEFINE_MANAGED_STATE now, might introduce more flags in future. If the flag is specified, the domain managed state file will be removed along with domain undefine process once it exists, the entire domain undefine process will fail if it fails on removing the managed state file. [PATCH 1/8] is small fix on rpc generator scripts, not related with the new API. [PATCH 1/8] Small fixes on rpc generator scripts [PATCH 2/8] UndefineFlags: Define the public API [PATCH 3/8] UndefineFlags: Implement the public API [PATCH 4/8] UndefineFlags: Define the internal API [PATCH 5/8] UndefineFlags: Implement qemu driver handling [PATCH 6/8] UndefineFlags: Implement libxl driver handling [PATCH 7/8] UndefineFlags: Implement the remote protocol [PATCH 8/8] UndefineFlags: Extend virsh undefine to support new flag Regards Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] Small fixes on rpc generator scripts
1. Fix typos caused by the renaming 2. It should substitute remote/remote_protocol.h to remote_protocol.h. --- src/rpc/gendispatch.pl |6 +++--- src/rpc/genprotocol.pl |2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index c69c5a2..39d0ce6 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -9,8 +9,8 @@ # for both remote_protocol.x and qemu_protocol.x, you would run the # following: # -# remote_generator.pl -c -t remote ../src/remote/remote_protocol.x -# remote_generator.pl -t qemu ../src/remote/qemu_protocol.x +# gendispatch.pl -c -t remote ../src/remote/remote_protocol.x +# gendispatch.pl -t qemu ../src/remote/qemu_protocol.x # # By Richard Jones rjo...@redhat.com # Extended by Matthias Bolte matthias.bo...@googlemail.com @@ -239,7 +239,7 @@ sub hyper_to_long # Output print __EOF__; -/* Automatically generated by remote_generator.pl. +/* Automatically generated by gendispatch.pl. * Do not edit this file. Any changes you make will be lost. */ __EOF__ diff --git a/src/rpc/genprotocol.pl b/src/rpc/genprotocol.pl index 4edba98..7124e5c 100755 --- a/src/rpc/genprotocol.pl +++ b/src/rpc/genprotocol.pl @@ -59,7 +59,7 @@ while (RPCGEN) { s/xdr_u_quad_t/xdr_uint64_t/g; s/xdr_quad_t/xdr_int64_t/g; s/(?!IXDR_GET_INT32 )IXDR_GET_LONG/IXDR_GET_INT32/g; -s,#include \./remote/remote_protocol\.h,#include remote_protocol.h,; +s,#include remote/remote_protocol\.h,#include remote_protocol.h,; if (m/^}/) { $in_function = 0; -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 02/27] maint: print flags in hex during debug
2011/7/8 Eric Blake ebl...@redhat.com: Continuation of commit 313ac7fd, and enforce things with a syntax check. Technically, virNetServerClientCalculateHandleMode is not printing a mode_t, but rather a collection of VIR_EVENT_HANDLE_* bits; however, these bits are 8, so there is no different in the output, and that was the easiest way to silence the new syntax check. * cfg.mk (sc_flags_debug): New syntax check. (exclude_file_name_regexp--sc_flags_debug): Add exemptions. * src/fdstream.c (virFDStreamOpenFileInternal): Print flags in hex, mode_t in octal. * src/libvirt-qemu.c (virDomainQemuMonitorCommand): Likewise. * src/locking/lock_driver_nop.c (virLockManagerNopInit): Likewise. * src/locking/lock_driver_sanlock.c (virLockManagerSanlockInit): Likewise. * src/locking/lock_manager.c: Likewise. * src/qemu/qemu_migration.c: Likewise. * src/qemu/qemu_monitor.c: Likewise. * src/rpc/virnetserverclient.c (virNetServerClientCalculateHandleMode): Print mode with %o. --- v2: new patch, first new syntax check cfg.mk | 11 +++ src/fdstream.c | 2 +- src/libvirt-qemu.c | 5 +++-- src/locking/lock_driver_nop.c | 3 ++- src/locking/lock_driver_sanlock.c | 3 ++- src/locking/lock_manager.c | 11 ++- src/qemu/qemu_migration.c | 16 src/qemu/qemu_monitor.c | 10 +- src/rpc/virnetserverclient.c | 2 +- 9 files changed, 39 insertions(+), 24 deletions(-) flags_debug src/libvirt-qemu.c:117:VIR_DEBUG(conn=%p, pid=%u, flags=%u, conn, pid, flags); maint.mk: debug flag values with %x Dan commited this after you posted your series. So ACK with that new offender fix too. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] UndefineFlags: Implement the remote protocol
With small typo fixes (s/remote_generator/gendispatch/, and s/remote_internal/remote_driver/) --- src/remote/qemu_protocol.x |2 +- src/remote/remote_driver.c |5 +++-- src/remote/remote_protocol.x | 14 ++ src/remote_protocol-structs |4 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x index fa453f4..9381f7c 100644 --- a/src/remote/qemu_protocol.x +++ b/src/remote/qemu_protocol.x @@ -43,7 +43,7 @@ const QEMU_PROTOCOL_VERSION = 1; enum qemu_procedure { /* Each function must have a two-word comment. The first word is - * whether remote_generator.pl handles daemon, the second whether + * whether gendispatch.pl handles daemon, the second whether * it handles src/remote. */ QEMU_PROC_MONITOR_COMMAND = 1 /* skipgen skipgen */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5c0457e..dafb04a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1,5 +1,5 @@ /* - * remote_internal.c: driver to provide access to libvirtd running + * remote_driver.c: driver to provide access to libvirtd running * on a remote machine * * Copyright (C) 2007-2011 Red Hat, Inc. @@ -4177,6 +4177,7 @@ static virDriver remote_driver = { .domainCreateWithFlags = remoteDomainCreateWithFlags, /* 0.8.2 */ .domainDefineXML = remoteDomainDefineXML, /* 0.3.0 */ .domainUndefine = remoteDomainUndefine, /* 0.3.0 */ +.domainUndefineWithFlags = remoteDomainUndefineWithFlags, /* 0.9.4 */ .domainAttachDevice = remoteDomainAttachDevice, /* 0.3.0 */ .domainAttachDeviceFlags = remoteDomainAttachDeviceFlags, /* 0.7.7 */ .domainDetachDevice = remoteDomainDetachDevice, /* 0.3.0 */ @@ -4244,7 +4245,7 @@ static virDriver remote_driver = { .domainMigratePerform3 = remoteDomainMigratePerform3, /* 0.9.2 */ .domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */ -.domainSendKey = remoteDomainSendKey, /* 0.9.3 */ +.domainSendKey = remoteDomainSendKey /* 0.9.3 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index ee169fd..3adaca8 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -339,7 +339,7 @@ struct remote_node_get_memory_stats { * connection). Errors are returned implicitly in the RPC protocol. * * Please follow the naming convention carefully - this file is - * parsed by 'remote_generator.pl'. + * parsed by 'gendispatch.pl'. * * 'remote_CALL_ret' members that are filled via call-by-reference must be * annotated with a insert@offset comment to indicate the offset in the @@ -848,6 +848,11 @@ struct remote_domain_undefine_args { remote_nonnull_domain dom; }; +struct remote_domain_undefine_with_flags_args { +remote_nonnull_domain dom; +unsigned int flags; +}; + struct remote_domain_inject_nmi_args { remote_nonnull_domain dom; unsigned int flags; @@ -2123,7 +2128,7 @@ const REMOTE_PROTOCOL_VERSION = 1; enum remote_procedure { /* Each function must have a two-word comment. The first word is - * whether remote_generator.pl handles daemon, the second whether + * whether gendispatch.pl handles daemon, the second whether * it handles src/remote. Additional flags can be specified after a * pipe. * @@ -2383,14 +2388,15 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_CPU_STATS = 227, /* skipgen skipgen */ REMOTE_PROC_NODE_GET_MEMORY_STATS = 228, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 229, /* autogen autogen */ -REMOTE_PROC_DOMAIN_GET_VCPU_PIN_INFO = 230 /* skipgen skipgen */ +REMOTE_PROC_DOMAIN_GET_VCPU_PIN_INFO = 230, /* skipgen skipgen */ +REMOTE_PROC_DOMAIN_UNDEFINE_WITH_FLAGS = 231 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? * Nice isn't it. Please keep it this way when adding more. * * Each function must have a two-word comment. The first word is - * whether remote_generator.pl handles daemon, the second whether + * whether gendispatch.pl handles daemon, the second whether * it handles src/remote. Additional flags can be specified after a * pipe. * diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index b2de8e9..c8fed2c 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -533,6 +533,10 @@ struct remote_domain_define_xml_ret { struct remote_domain_undefine_args { remote_nonnull_domain dom; }; +struct remote_domain_undefine_with_flags_args { +remote_nonnull_domain dom; +u_int flags; +}; struct remote_domain_inject_nmi_args { remote_nonnull_domain dom; u_int flags; -- 1.7.6 --
[libvirt] [PATCH 4/8] UndefineFlags: Define the internal API
--- src/driver.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index 71a52c9..3db9fb7 100644 --- a/src/driver.h +++ b/src/driver.h @@ -219,6 +219,9 @@ typedef virDomainPtr typedef int (*virDrvDomainUndefine)(virDomainPtr dom); typedef int +(*virDrvDomainUndefineWithFlags) (virDomainPtr dom, + unsigned int flags); +typedef int (*virDrvDomainSetVcpus)(virDomainPtr domain, unsigned int nvcpus); typedef int @@ -727,6 +730,7 @@ struct _virDriver { virDrvDomainCreateWithFlagsdomainCreateWithFlags; virDrvDomainDefineXML domainDefineXML; virDrvDomainUndefinedomainUndefine; +virDrvDomainUndefineWithFlags domainUndefineWithFlags; virDrvDomainAttachDevice domainAttachDevice; virDrvDomainAttachDeviceFlags domainAttachDeviceFlags; virDrvDomainDetachDevice domainDetachDevice; -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] UndefineFlags: Implement libxl driver handling
--- src/libxl/libxl_driver.c | 32 1 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 808480f..f04931b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -220,10 +220,8 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) static char * libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { char *ret; -char uuidstr[VIR_UUID_STRING_BUFLEN]; -virUUIDFormat(vm-def-uuid, uuidstr); -if (virAsprintf(ret, %s/%s.save, driver-saveDir, uuidstr) 0) { +if (virAsprintf(ret, %s/%s.save, driver-saveDir, vm-def-name) 0) { virReportOOMError(); return NULL; } @@ -2716,13 +2714,17 @@ cleanup: } static int -libxlDomainUndefine(virDomainPtr dom) +libxlDomainUndefineWithFlags(virDomainPtr dom, + unsigned int flags) { libxlDriverPrivatePtr driver = dom-conn-privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; +char *name = NULL; int ret = -1; +virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_STATE, -1); + libxlDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); @@ -2746,6 +2748,20 @@ libxlDomainUndefine(virDomainPtr dom) %s, _(cannot undefine transient domain)); goto cleanup; } + +if (flags VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { +name = libxlDomainManagedSavePath(driver, vm); + +if (name == NULL) +goto cleanup; + +if (virFileExists(name) (unlink(name) 0)) { +libxlError(VIR_ERR_INTERNAL_ERROR, + _(failed on removing domain managed state + file '%s'), name); +goto cleanup; +} +} if (virDomainDeleteConfig(driver-configDir, driver-autostartDir, @@ -2760,6 +2776,7 @@ libxlDomainUndefine(virDomainPtr dom) ret = 0; cleanup: +VIR_FREE(name); if (vm) virDomainObjUnlock(vm); if (event) @@ -2769,6 +2786,12 @@ libxlDomainUndefine(virDomainPtr dom) } static int +libxlDomainUndefine(virDomainPtr dom) +{ +return libxlDomainUndefineWithFlags(dom, 0); +} + +static int libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainDiskDefPtr disk) { @@ -3828,6 +3851,7 @@ static virDriver libxlDriver = { .domainCreateWithFlags = libxlDomainCreateWithFlags, /* 0.9.0 */ .domainDefineXML = libxlDomainDefineXML, /* 0.9.0 */ .domainUndefine = libxlDomainUndefine, /* 0.9.0 */ +.domainUndefineWithFlags = libxlDomainUndefineWithFlags, /* 0.9.4 */ .domainAttachDevice = libxlDomainAttachDevice, /* 0.9.2 */ .domainAttachDeviceFlags = libxlDomainAttachDeviceFlags, /* 0.9.2 */ .domainDetachDevice = libxlDomainDetachDevice,/* 0.9.2 */ -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] UndefineFlags: Extend virsh undefine to support new flag
--- tools/virsh.c | 12 +++- tools/virsh.pod |6 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3cdf043..f81e923 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1409,6 +1409,7 @@ static const vshCmdInfo info_undefine[] = { static const vshCmdOptDef opts_undefine[] = { {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name or uuid)}, +{undefine-managed-state, VSH_OT_BOOL, 0, N_(remove domain managed state file)}, {NULL, 0, 0, NULL} }; @@ -1419,6 +1420,14 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) bool ret = true; const char *name = NULL; int id; +int flags = 0; +int undefine_managed_state = vshCommandOptBool(cmd, undefine-managed-state); + +if (undefine_managed_state) +flags |= VIR_DOMAIN_UNDEFINE_MANAGED_STATE; + +if (!undefine_managed_state) +flags = -1; if (!vshConnectionUsability(ctl, ctl-conn)) return false; @@ -1440,7 +1449,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) VSH_BYNAME|VSH_BYUUID))) return false; -if (virDomainUndefine(dom) == 0) { +if (((flags == -1) ? virDomainUndefine(dom) : +virDomainUndefineWithFlags(dom, flags)) == 0) { vshPrint(ctl, _(Domain %s has been undefined\n), name); } else { vshError(ctl, _(Failed to undefine domain %s), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index 8b820d2..393d014 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -804,11 +804,15 @@ hypervisor. Output the device used for the TTY console of the domain. If the information is not available the processes will provide an exit code of 1. -=item Bundefine Idomain-id +=item Bundefine Idomain-id optional I--undefine-managed-state Undefine the configuration for an inactive domain. Since it's not running the domain name or UUID must be used as the Idomain-id. +If I--undefine-managed-state is specified, the managed state file will +be removed along with the domain undefine peocess, the entire domain +undefine process will fail if it fails on removing the managed state file. + =item Bvcpucount Idomain-id optional I--maximum I--current I--config I--live -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/8] UndefineFlags: Implement the public API
--- src/libvirt.c | 53 - 1 files changed, 52 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 7e70caa..eeb25a5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6381,7 +6381,8 @@ error: * Returns 0 in case of success, -1 in case of error */ int -virDomainUndefine(virDomainPtr domain) { +virDomainUndefine(virDomainPtr domain) +{ virConnectPtr conn; VIR_DOMAIN_DEBUG(domain); @@ -6415,6 +6416,56 @@ error: } /** + * virDomainUndefineWithFlags: + * @domain: pointer to a defined domain + * @flags: bitwise-or of supported virDomainUndefineFlags + * + * Undefine a domain but does not stop it if it is running + * + * If VIR_DOMAIN_UNDEFINE_MANAGED_STATE is specified, the domain + * managed state file will be removed along with domain undefine + * process, the entire domain undefine process will fail if + * it fails on removing the managed state file. + * + * Returns 0 in case of success, -1 in case of error + */ +int +virDomainUndefineWithFlags(virDomainPtr domain, + unsigned int flags) +{ +virConnectPtr conn; + +VIR_DOMAIN_DEBUG(domain, flags=%x, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN(domain)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} +conn = domain-conn; +if (conn-flags VIR_CONNECT_RO) { +virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (conn-driver-domainUndefineWithFlags) { +int ret; +ret = conn-driver-domainUndefineWithFlags (domain, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(domain-conn); +return -1; +} + +/** * virConnectNumOfDefinedDomains: * @conn: pointer to the hypervisor connection * -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] UndefineFlags: Implement qemu driver handling
--- src/qemu/qemu_driver.c | 31 +++ 1 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f962e2c..a9f6986 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2402,10 +2402,8 @@ cleanup: static char * qemuDomainManagedSavePath(struct qemud_driver *driver, virDomainObjPtr vm) { char *ret; -char uuidstr[VIR_UUID_STRING_BUFLEN]; -virUUIDFormat(vm-def-uuid, uuidstr); -if (virAsprintf(ret, %s/%s.save, driver-saveDir, uuidstr) 0) { +if (virAsprintf(ret, %s/%s.save, driver-saveDir, vm-def-name) 0) { virReportOOMError(); return(NULL); } @@ -4283,11 +4281,16 @@ cleanup: return dom; } -static int qemudDomainUndefine(virDomainPtr dom) { +static int qemudDomainUndefineWithFlags(virDomainPtr dom, +unsigned int flags) +{ struct qemud_driver *driver = dom-conn-privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; int ret = -1; +char *name = NULL; + +virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_STATE, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); @@ -4312,6 +4315,20 @@ static int qemudDomainUndefine(virDomainPtr dom) { goto cleanup; } +if (flags VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { +name = qemuDomainManagedSavePath(driver, vm); + +if (name == NULL) +goto cleanup; + +if (virFileExists(name) (unlink(name) 0)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(failed on removing domain managed state +file '%s'), name); +goto cleanup; +} +} + if (virDomainDeleteConfig(driver-configDir, driver-autostartDir, vm) 0) goto cleanup; @@ -4326,6 +4343,7 @@ static int qemudDomainUndefine(virDomainPtr dom) { ret = 0; cleanup: +VIR_FREE(name); if (vm) virDomainObjUnlock(vm); if (event) @@ -4334,6 +4352,10 @@ cleanup: return ret; } +static int qemudDomainUndefine(virDomainPtr dom) { +return qemudDomainUndefineWithFlags(dom, 0); +} + static int qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver, virDomainObjPtr vm, @@ -8487,6 +8509,7 @@ static virDriver qemuDriver = { .domainCreateWithFlags = qemudDomainStartWithFlags, /* 0.8.2 */ .domainDefineXML = qemudDomainDefine, /* 0.2.0 */ .domainUndefine = qemudDomainUndefine, /* 0.2.0 */ +.domainUndefineWithFlags = qemudDomainUndefineWithFlags, /* 0.9.4 */ .domainAttachDevice = qemuDomainAttachDevice, /* 0.4.1 */ .domainAttachDeviceFlags = qemuDomainAttachDeviceFlags, /* 0.7.7 */ .domainDetachDevice = qemuDomainDetachDevice, /* 0.5.0 */ -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 04/27] libvirt-qemu: use unsigned flags
2011/7/8 Eric Blake ebl...@redhat.com: Like commit 1740c381, but for libvirt-qemu. * src/remote/qemu_protocol.x (qemu_monitor_command_args): Adjust type to match API. * src/qemu_protocol-structs: Update accordingly. --- v2: new patch src/qemu_protocol-structs | 2 +- src/remote/qemu_protocol.x | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) Should we have a syntax-check rule to enforce unsigned int flags in the RPC protocol like you added a rule to do this in the public API. ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 06/27] node_device: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/node_device/node_device_driver.c (nodeNumOfDevices) (nodeListDevices, nodeDeviceGetXMLDesc, nodeDeviceCreateXML): Reject unknown flags. * src/node_device/node_device_hal.c (halNodeDrvOpen): Likewise. * src/node_device/node_device_udev.c (udevNodeDrvOpen): Likewise. --- src/node_device/node_device_driver.c | 18 +- src/node_device/node_device_hal.c | 4 +++- src/node_device/node_device_udev.c | 4 +++- 3 files changed, 19 insertions(+), 7 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 05/27] util: reject unknown flags, and prefer unsigned flags
2011/7/8 Eric Blake ebl...@redhat.com: Silently ignored flags get in the way of new features that use those flags. Also, an upcoming syntax check will favor unsigned flags. * src/nodeinfo.h (nodeGetCPUStats, nodeGetMemoryStats): Drop unused attribute. * src/interface/netcf_driver.c (interfaceOpenInterface) (interfaceDefineXML, interfaceCreate, interfaceDestroy): Reject unknown flags. * src/network/bridge_driver.c (networkOpenNetwork) (networkGetXMLDesc): Likewise. * src/nwfilter/nwfilter_driver.c (nwfilterOpen): Likewise. * src/secret/secret_driver.c (secretOpen, secretDefineXML) (secretGetXMLDesc, secretSetValue): Likewise. * src/util/logging.c (virLogDefineFilter, virLogDefineOutput) (virLogMessage): Likewise; also use unsigned flags. * src/util/logging.h (virLogDefineFilter, virLogDefineOutput) (virLogMessage): Change signature. * src/util/command.c (virExecWithHook): Likewise. --- ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 07/27] storage: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/storage/storage_backend.c (virStorageBackendCreateBlockFrom) (virStorageBackendCreateQemuImg) (virStorageBackendCreateQcowCreate): Reject unknown flags. * src/storage/storage_backend_disk.c (virStorageBackendDiskBuildPool) (virStorageBackendDiskDeleteVol): Likewise. * src/storage/storage_backend_fs.c (virStorageBackendFileSystemNetFindPoolSources) (virStorageBackendFileSystemBuild) (virStorageBackendFileSystemDelete, createFileDir) (virStorageBackendFileSystemVolBuildFrom) (virStorageBackendFileSystemVolDelete): Likewise. * src/storage/storage_backend_iscsi.c (virStorageBackendISCSIFindPoolSources): Likewise. * src/storage/storage_backend_logical.c (virStorageBackendLogicalFindPoolSources) (virStorageBackendLogicalBuildPool) (virStorageBackendLogicalDeletePool) (virStorageBackendLogicalDeleteVol): Likewise. * src/storage/storage_driver.c (storageOpen, storagePoolCreate) (storagePoolDefine, storagePoolRefresh, storagePoolGetXMLDesc) (storageVolumeCreateXML, storageVolumeCreateXMLFrom) (storageVolumeGetXMLDesc): Likewise. --- src/storage/storage_backend.c | 12 ++-- src/storage/storage_backend_disk.c | 10 +-- src/storage/storage_backend_fs.c | 26 ++ src/storage/storage_backend_iscsi.c | 4 ++- src/storage/storage_backend_logical.c | 18 +--- src/storage/storage_driver.c | 45 ++-- 6 files changed, 88 insertions(+), 27 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 08/27] esx: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: Silently ignored flags get in the way of new features that use those flags. * src/esx/esx_device_monitor.c (esxDeviceOpen): Reject unknown flags. * src/esx/esx_driver.c (esxOpen, esxDomainReboot) (esxDomainXMLFromNative, esxDomainXMLToNative) (esxDomainMigratePrepare, esxDomainMigratePerform) (esxDomainMigrateFinish): Likewise. * src/esx/esx_interface_driver.c (esxInterfaceOpen): Likewise. * src/esx/esx_network_driver.c (esxNetworkOpen): Likewise. * src/esx/esx_nwfilter_driver.c (esxNWFilterOpen): Likewise. * src/esx/esx_secret_driver.c (esxSecretOpen): Likewise. * src/esx/esx_storage_driver.c (esxStorageOpen): Likewise. --- src/esx/esx_device_monitor.c | 4 +++- src/esx/esx_driver.c | 28 +--- src/esx/esx_interface_driver.c | 4 +++- src/esx/esx_network_driver.c | 4 +++- src/esx/esx_nwfilter_driver.c | 4 +++- src/esx/esx_secret_driver.c | 4 +++- src/esx/esx_storage_driver.c | 4 +++- 7 files changed, 39 insertions(+), 13 deletions(-) @@ -3829,12 +3837,14 @@ esxDomainMigratePrepare(virConnectPtr dconn, int *cookielen ATTRIBUTE_UNUSED, const char *uri_in ATTRIBUTE_UNUSED, char **uri_out, - unsigned long flags ATTRIBUTE_UNUSED, + unsigned long flags, const char *dname ATTRIBUTE_UNUSED, unsigned long resource ATTRIBUTE_UNUSED) { esxPrivate *priv = dconn-privateData; + virCheckFlags(0, -1); + if (uri_in == NULL) { if (virAsprintf(uri_out, vpxmigr://%s/%s/%s, priv-vCenter-ipAddress, @@ -3855,7 +3865,7 @@ esxDomainMigratePerform(virDomainPtr domain, const char *cookie ATTRIBUTE_UNUSED, int cookielen ATTRIBUTE_UNUSED, const char *uri, - unsigned long flags ATTRIBUTE_UNUSED, + unsigned long flags, const char *dname, unsigned long bandwidth ATTRIBUTE_UNUSED) { @@ -3873,6 +3883,8 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; + virCheckFlags(0, -1); + if (priv-vCenter == NULL) { ESX_ERROR(VIR_ERR_INVALID_ARG, %s, _(Migration not possible without a vCenter)); @@ -4001,8 +4013,10 @@ esxDomainMigrateFinish(virConnectPtr dconn, const char *dname, const char *cookie ATTRIBUTE_UNUSED, int cookielen ATTRIBUTE_UNUSED, const char *uri ATTRIBUTE_UNUSED, - unsigned long flags ATTRIBUTE_UNUSED) + unsigned long flags) { + virCheckFlags(0, NULL); + return esxDomainLookupByName(dconn, dname); } Actually this is wrong. This was implemented before libvirt knew about VIR_MIGRATE_PERSIST_DEST VIR_MIGRATE_UNDEFINE_SOURCE But an ESX migration has exactly the semantic implied by those two flags. So actually those two flags have to be passed always. I should have fixed that ages ago. Also if the domain is running ESX does a live migration always, so VIR_MIGRATE_LIVE has to passed too. ESX supports migration of inactive domains. It seems that migration in libvirt is only meant for active domains, but this isn't documented well. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 09/27] libxl: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/libxl/libxl_driver.c (libxlOpen, libxlDomainReboot) (libxlDomainXMLFromNative, libxlDomainXMLToNative) (libxlDomainCreateWithFlags): Reject unknown flags. --- src/libxl/libxl_driver.c | 18 +- 1 files changed, 13 insertions(+), 5 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/27] lxc: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/lxc/lxc_driver.c (lxcOpen, lxcDomainSetMemoryParameters) (lxcDomainGetMemoryParameters): Reject unknown flags. * src/lxc/lxc_container.c (lxcContainerStart): Use unsigned flags. --- src/lxc/lxc_container.c | 4 ++-- src/lxc/lxc_driver.c | 12 +--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index ef8469c..ffa2f34 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2010 Red Hat, Inc. + * Copyright (C) 2008-2011 Red Hat, Inc. * Copyright (C) 2008 IBM Corp. * * lxc_container.c: file description @@ -889,7 +889,7 @@ int lxcContainerStart(virDomainDefPtr def, char *ttyPath) { pid_t pid; - int flags; + unsigned int flags; flags is passed to clone that takes int, so rename this to cflags (in line with oflags and fflags) and keep it as int. ACK, with the clone flags as int. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 11/27] openvz: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/openvz/openvz_driver.c (openvzDomainReboot, openvzOpen): Reject unknown flags. --- src/openvz/openvz_driver.c | 9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 12/27] phyp: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/phyp/phyp_driver.c (phypOpen, phypDomainReboot) (phypVIOSDriverOpen): Reject unknown flags. --- src/phyp/phyp_driver.c | 12 +--- 1 files changed, 9 insertions(+), 3 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 13/27] qemu: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/qemu/qemu_driver.c (qemudOpen, qemuDomainScreenshot) (qemuDomainXMLFromNative, qemuDomainXMLToNative) (qemudDomainBlockPeek, qemuCPUCompare, qemuCPUBaseline): Reject unknown flags. * src/qemu/qemu_migration.c (qemuMigrationConfirm): Likewise. (_qemuMigrationCookie, qemuMigrationCookieXMLParse) (qemuMigrationCookieXMLParseStr, qemuMigrationBakeCookie) (qemuMigrationEatCookie): Make flags unsigned. * src/qemu/qemu_domain.h: (qemuDomainDefFormatXML) (qemuDomainFormatXML): Prefer unsigned flags. * src/qemu/qemu_domain.c (qemuDomainDefFormatXML) (qemuDomainFormatXML): Likewise. (qemuDomainOpenLogHelper, qemuDomainCreateLog): Rename variable. --- src/qemu/qemu_domain.c | 21 +++-- src/qemu/qemu_domain.h | 4 ++-- src/qemu/qemu_driver.c | 30 +++--- src/qemu/qemu_migration.c | 16 +--- 4 files changed, 45 insertions(+), 26 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 14/27] test: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/test/test_driver.c (testOpen, testDomainCoreDump) (testOpenNetwork, testNetworkGetXMLDesc, testOpenInterface) (testInterfaceChangeBegin, testInterfaceChangeCommit) (testInterfaceChangeRollback, testInterfaceGetXMLDesc) (testInterfaceDefineXML, testInterfaceCreate) (testInterfaceDestroy, testStorageOpen, testStoragePoolStart) (testStorageFindPoolSources, testStoragePoolCreate) (testStoragePoolDefine, testStoragePoolBuild) (testStoragePoolDelete, testStoragePoolRefresh) (testStoragePoolGetXMLDesc, testStorageVolumeCreateXML) (testStorageVolumeCreateXMLFrom, testStorageVolumeDelete) (testStorageVolumeGetXMLDesc, testDevMonOpen) (testNodeNumOfDevices, testNodeListDevices) (testNodeDeviceGetXMLDesc, testNodeDeviceCreateXML) (testSecretOpen, testNWFilterOpen): Reject unknown flags. --- src/test/test_driver.c | 144 +--- 1 files changed, 112 insertions(+), 32 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 98daca8..5ff01a3 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1099,11 +1099,13 @@ static int testOpenFromFile(virConnectPtr conn, static virDrvOpenStatus testOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { int ret; testConnPtr privconn; + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + if (!conn-uri) return VIR_DRV_OPEN_DECLINED; @@ -1904,7 +1906,7 @@ cleanup: static int testDomainCoreDump(virDomainPtr domain, const char *to, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { testConnPtr privconn = domain-conn-privateData; int fd = -1; @@ -1912,6 +1914,8 @@ static int testDomainCoreDump(virDomainPtr domain, virDomainEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); testDomainCoreDump understands VIR_DUMP_CRASH. Don't get fooled by the ATTRIBUTE_UNUSED :) ACK, with testDomainCoreDump fixed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 15/27] uml: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/uml/uml_driver.c (umlOpen, umlDomainGetXMLDesc) (umlDomainBlockPeek): Reject unknown flags. --- src/uml/uml_driver.c | 14 +++--- 1 files changed, 11 insertions(+), 3 deletions(-) @@ -1559,11 +1562,14 @@ cleanup: static char *umlDomainGetXMLDesc(virDomainPtr dom, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) +{ struct uml_driver *driver = dom-conn-privateData; virDomainObjPtr vm; char *ret = NULL; + virCheckFlags(0, NULL); + umlDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); umlDriverUnlock(driver); Don't get fooled by the ATTRIBUTE_UNUSED again. All *DomainGetXMLDesc use virDomainDefFormat and have to accept all flags that virDomainDefFormat accepts. I suggest to recheck your series for this pattern, here it's just the first time that I notice it. ACK, with that virCheckFlags loosened correctly. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 16/27] vbox: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/vbox/vbox_driver.c (vboxOpenDummy): Reject unknown flags. * src/vbox/vbox_tmpl.c (vboxOpen, vboxDomainReboot) (vboxNetworkOpen, vboxNetworkGetXMLDesc, vboxStorageOpen) (vboxStorageVolCreateXML, vboxStorageVolDelete) (vboxStorageVolGetXMLDesc, vboxDomainScreenshot): Likewise. --- src/vbox/vbox_driver.c | 5 - src/vbox/vbox_tmpl.c | 44 +++- 2 files changed, 39 insertions(+), 10 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 17/27] vmware: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/vmware/vmware_driver.c (vmwareOpen, vmwareDomainReboot) (vmwareDomainCreateXML, vmwareDomainCreateWithFlags): Reject unknown flags. --- src/vmware/vmware_driver.c | 17 - 1 files changed, 12 insertions(+), 5 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix missing prompt message for 'snapshot-delete' command
于 2011年07月13日 16:36, Nan Zhang 写道: Make the command 'virsh snapshot-delete' has the appropriate prompt message when executing sucessful or failed. --- tools/virsh.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index cd17f42..b7cea58 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11534,8 +11534,12 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) if (snapshot == NULL) goto cleanup; -if (virDomainSnapshotDelete(snapshot, flags) 0) You can keep this line unchanged, and switch the prompt strings for failure and success. But it's fine, ACK +if (virDomainSnapshotDelete(snapshot, flags) == 0) { +vshPrint(ctl, _(Domain snapshot %s deleted\n), name); +} else { +vshError(ctl, _(Failed to delete snapshot %s), name); goto cleanup; +} ret = true; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 18/27] xen: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/xen/xen_driver.c (xenUnifiedDomainXMLFromNative) (xenUnifiedDomainXMLToNative, xenUnifiedDomainBlockPeek): Reject unknown flags. * src/xen/xen_hypervisor.c (xenHypervisorOpen) (xenHypervisorGetDomainState): Likewise. * src/xen/xen_inotify.c (xenInotifyOpen): Likewise. * src/xen/xs_internal.c (xenStoreOpen, xenStoreDomainGetState) (xenStoreDomainReboot): Likewise. * src/xen/xend_internal.c (xenDaemonOpen, xenDaemonDomainReboot) (xenDaemonDomainCoreDump, xenDaemonDomainGetState) (xenDaemonDomainMigratePrepare): Likewise. (xenDaemonDomainGetXMLDesc): Prefer unsigned flags. * src/xen/xend_internal.h (xenDaemonDomainGetXMLDesc): Likewise. * src/xen/xm_internal.h (xenXMDomainGetXMLDesc): Likewise. * src/xen/xm_internal.c (xenXMDomainGetXMLDesc): Likewise. (xenXMOpen, xenXMDomainGetState): Reject unknown flags. --- src/xen/xen_driver.c | 12 +--- src/xen/xen_hypervisor.c | 8 ++-- src/xen/xen_inotify.c | 4 +++- src/xen/xend_internal.c | 23 +-- src/xen/xend_internal.h | 3 ++- src/xen/xm_internal.c | 11 --- src/xen/xm_internal.h | 2 +- src/xen/xs_internal.c | 12 +--- 8 files changed, 55 insertions(+), 20 deletions(-) @@ -1629,8 +1633,10 @@ xenDaemonDomainSave(virDomainPtr domain, const char *filename) */ static int xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { + virCheckFlags(0, -1); + if ((domain == NULL) || (domain-conn == NULL) || (domain-name == NULL) || (filename == NULL)) { virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__); Third time fooled by ATTRIBUTE_UNUSED. xenDaemonDomainCoreDump understands VIR_DUMP_LIVE and VIR_DUMP_CRASH. @@ -3151,10 +3160,12 @@ xenDaemonDomainMigratePrepare (virConnectPtr dconn, int *cookielen ATTRIBUTE_UNUSED, const char *uri_in, char **uri_out, - unsigned long flags ATTRIBUTE_UNUSED, + unsigned long flags, const char *dname ATTRIBUTE_UNUSED, unsigned long resource ATTRIBUTE_UNUSED) { + virCheckFlags(0, -1); + /* If uri_in is NULL, get the current hostname as a best guess * of how the source host should connect to us. Note that caller * deallocates this string. This breaks virDomainMigrate, because *DomainMigratePrepare is called with the flags passed to virDomainMigrate, even if xenDaemonDomainMigratePrepare doesn't use this flags it has to accept all flags that the Xen driver understand in general form migration. A quick grep for VIR_MIGRATE_ shows that those are at least VIR_MIGRATE_LIVE VIR_MIGRATE_PERSIST_DEST VIR_MIGRATE_UNDEFINE_SOURCE VIR_MIGRATE_PAUSED -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 19/27] xenapi: reject unknown flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/xenapi/xenapi_driver.c (xenapiOpen, xenapiDomainReboot) (xenapiDomainGetXMLDesc): Reject unknown flags. --- src/xenapi/xenapi_driver.c | 13 ++--- 1 files changed, 10 insertions(+), 3 deletions(-) @@ -1309,6 +1314,8 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) struct xen_vif_set *vif_set = NULL; char *xml; + virCheckFlags(0, NULL); + if (!xen_vm_get_by_name_label(session, vms, dom-name)) return NULL; if (vms-size != 1) { xenapiSessionErrorHandler(dom-conn, VIR_ERR_INTERNAL_ERROR, You found a bug, but made it worse instead of fixing it. xenapiDomainGetXMLDesc should pass the flags to virDomainDefFormat instead of passing 0. ACK, with passing flags to virDomainDefFormat instead of ignoring it. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 23/27] conf: prefer unsigned flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/conf/cpu_conf.h (virCPUDefFormat, virCPUDefFormatBuf): Change flags type. * src/conf/cpu_conf.c (virCPUDefFormat, virCPUDefFormatBuf): Likewise. * src/conf/storage_conf.c (_virStoragePoolOptions): Likewise. * src/datatypes.h (_virConnect, _virStream): Likewise. --- v2: new patch src/conf/cpu_conf.c | 6 +++--- src/conf/cpu_conf.h | 6 +++--- src/conf/storage_conf.c | 4 ++-- src/datatypes.h | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 20/27] virsh, daemon: prefer unsigned flags
2011/7/8 Eric Blake ebl...@redhat.com: * tools/virsh.c (vshCmdDef): Change flags type. * daemon/remote.c (remoteDispatchOpen): Likewise. --- v2: new patch daemon/remote.c | 2 +- tools/virsh.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 22/27] python: prefer unsigned flags
2011/7/8 Eric Blake ebl...@redhat.com: * python/libvirt-override.c (libvirt_virConnectOpenAuth) (libvirt_virDomainSnapshotListNames) (libvirt_virDomainRevertToSnapshot): Change flags type. --- v2: new patch python/libvirt-override.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 21/27] node_device: avoid implicit int
2011/7/8 Eric Blake ebl...@redhat.com: 'unsigned a' and 'unsigned int a' are synonyms, but we generally always spell out the 'int' in that case. Fixing this will avoid a false positive in the next syntax-check commit. * src/conf/node_device_conf.h (pci_config_address) (_virNodeDevCapsDef): Prefer 'unsigned int' over 'unsigned'. --- v2: new patch. I didn't add a syntax check for implicit int, though, as we have a lot of other cases of that. Rather, this was the only hit when grepping for 'unsigned flags'. src/conf/node_device_conf.h | 58 +- 1 files changed, 29 insertions(+), 29 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 25/27] conf: delete unused flags arguments
2011/7/8 Eric Blake ebl...@redhat.com: For static functions not used as callbacks, there's no need to keep an unused parameter. * src/conf/domain_conf.c (virDomainChrDefParseTargetXML) (virDomainTimerDefParseXML, virDomainHostdevSubsysUsbDefParseXML) (virDomainVcpuPinDefParseXML): Drop unused parameter. (virDomainChrDefParseXML, virDomainDefParseXML) (virDomainHostdevDefParseXML): Update callers. (virDomainNetDefParseXML): Mark flags used. --- v2: new patch src/conf/domain_conf.c | 24 +--- 1 files changed, 9 insertions(+), 15 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 24/27] build: don't hand-roll cloexec code
2011/7/8 Eric Blake ebl...@redhat.com: No need to repeat common code. * src/util/bridge.c (brInit): Use virSetCloseExec. (brSetInterfaceUp): Prefer unsigned flags. * src/uml/uml_driver.c (umlSetCloseExec): Delete. (umlStartVMDaemon): Use util version instead. --- v2: new patch src/uml/uml_driver.c | 19 +++ src/util/bridge.c | 19 +-- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/src/util/bridge.c b/src/util/bridge.c index 7204e64..a6b5768 100644 --- a/src/util/bridge.c +++ b/src/util/bridge.c @@ -72,25 +72,16 @@ int brInit(brControl **ctlp) { int fd; - int flags; if (!ctlp || *ctlp) return EINVAL; fd = socket(AF_INET, SOCK_STREAM, 0); - if (fd 0) - return errno; - - if ((flags = fcntl(fd, F_GETFD)) 0 || - fcntl(fd, F_SETFD, flags | FD_CLOEXEC) 0) { - int err = errno; - VIR_FORCE_CLOSE(fd); - return err; - } - - if (VIR_ALLOC(*ctlp) 0) { + if (fd 0 || + virSetCloseExec(fd) 0 || + VIR_ALLOC(*ctlp) 0) { VIR_FORCE_CLOSE(fd); - return ENOMEM; + return errno; } Is it guaranteed that calloc will set ENOMEM, or do we need some gnulib module to guarantee this? (*ctlp)-fd = fd; @@ -599,7 +590,7 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; - int flags; + unsigned int flags; flags is used used with ifr.ifr_flags that is signed (actually it's a short). So I'd prefer renaming it to ifflags and keep it as int. ACK, with that questions/comments answered/addressed. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 26/27] remote: prefer unsigned flags
2011/7/8 Eric Blake ebl...@redhat.com: * src/remote/remote_driver.c (call, remoteOpenSecondaryDriver): Prefer unsigned flags. --- v2: new patch src/remote/remote_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/8] UndefineFlags: Extend virsh undefine to support new flag
On Wed, Jul 13, 2011 at 06:19:44PM +0800, Osier Yang wrote: --- tools/virsh.c | 12 +++- tools/virsh.pod |6 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3cdf043..f81e923 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1409,6 +1409,7 @@ static const vshCmdInfo info_undefine[] = { static const vshCmdOptDef opts_undefine[] = { {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name or uuid)}, +{undefine-managed-state, VSH_OT_BOOL, 0, N_(remove domain managed state file)}, IMO, it would be clearer to call this option remove-managed-state. Dave {NULL, 0, 0, NULL} }; @@ -1419,6 +1420,14 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) bool ret = true; const char *name = NULL; int id; +int flags = 0; +int undefine_managed_state = vshCommandOptBool(cmd, undefine-managed-state); + +if (undefine_managed_state) +flags |= VIR_DOMAIN_UNDEFINE_MANAGED_STATE; + +if (!undefine_managed_state) +flags = -1; if (!vshConnectionUsability(ctl, ctl-conn)) return false; @@ -1440,7 +1449,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) VSH_BYNAME|VSH_BYUUID))) return false; -if (virDomainUndefine(dom) == 0) { +if (((flags == -1) ? virDomainUndefine(dom) : +virDomainUndefineWithFlags(dom, flags)) == 0) { vshPrint(ctl, _(Domain %s has been undefined\n), name); } else { vshError(ctl, _(Failed to undefine domain %s), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index 8b820d2..393d014 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -804,11 +804,15 @@ hypervisor. Output the device used for the TTY console of the domain. If the information is not available the processes will provide an exit code of 1. -=item Bundefine Idomain-id +=item Bundefine Idomain-id optional I--undefine-managed-state Undefine the configuration for an inactive domain. Since it's not running the domain name or UUID must be used as the Idomain-id. +If I--undefine-managed-state is specified, the managed state file will +be removed along with the domain undefine peocess, the entire domain +undefine process will fail if it fails on removing the managed state file. + =item Bvcpucount Idomain-id optional I--maximum I--current I--config I--live -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: honor anchored names when searching for executables
On 07/12/2011 08:51 PM, Daniel Veillard wrote: On Tue, Jul 12, 2011 at 04:51:51PM -0600, Eric Blake wrote: I got bit in a debugging session on an uninstalled libvirtd; the code tried to call out to the installed $LIBEXECDIR/libvirt_iohelper instead of my just-built version. So I set a breakpoint and altered the binary name to be ./src/libvirt_iohelper, and it still failed because I don't have . on my PATH. According to POSIX, execvp only searches PATH if the name does not contain a slash. Since we are trying to mimic that behavior, an anchored name should be relative to the current working dir. +/* If we are passed an anchored path (containing a /), then there + * is no path search - it must exist in the current directory + */ +if (strchr(file, '/')) { +virFileAbsPath(file, path); +return path; +} + /* copy PATH env so we can tweak it */ path = getenv(PATH); That sounds right. The only issue is that the slight change of semantic may suddenly allow to run binaries outside of $PATH which may be a security concern. You can already run binaries outside of $PATH by giving an absolute name. All this does is actually avoid the PATH search on anchored relative names. But virFindFileInPath() shouldn't be the place to implement such a security control, so ACK, I realized that I missed two pieces - virFileAbsPath is attribute-warn-unused, and the file also has to be executable. I squashed this in, then pushed. diff --git i/src/util/util.c w/src/util/util.c index 1654cc2..08c8050 100644 --- i/src/util/util.c +++ w/src/util/util.c @@ -578,7 +578,7 @@ int virFileResolveLink(const char *linkpath, */ char *virFindFileInPath(const char *file) { -char *path; +char *path = NULL; char *pathiter; char *pathseg; char *fullpath = NULL; @@ -600,7 +600,8 @@ char *virFindFileInPath(const char *file) * is no path search - it must exist in the current directory */ if (strchr(file, '/')) { -virFileAbsPath(file, path); +if (virFileIsExecutable(file)) +ignore_value(virFileAbsPath(file, path)); return path; } -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 27/27] build: add syntax check for proper flags use
2011/7/8 Eric Blake ebl...@redhat.com: Enforce the recent flags cleanups - we want to use 'unsigned int flags' in any of our APIs (except where backwards compatibility is important, in the public migration APIs), and that all flags are checked for validity (except when there are stub functions that completely ignore the flags argument). There are a few minor tweaks done here to avoid false positives: signed arguments passed to open() are renamed oflags, and flags arguments that are legitimately ignored are renamed flags_unused. * cfg.mk (sc_flags_usage): New rule. (exclude_file_name_regexp--sc_flags_usage): And a few exemptions. * src/util/iohelper.c (runIO, main): Rename variable. * src/util/util.c (virSetInherit): Likewise. * src/fdstream.h (virFDStreamOpenFile, virFDStreamCreateFile): Likewise. * src/fdstream.c (virFDStreamOpenFileInternal) (virFDStreamOpenFile, virFDStreamCreateFile): Likewise. * src/util/command.c (virExecWithHook) [WIN32]: Likewise. * src/util/util.c (virFileOpenAs, virDirCreate) [WIN32]: Likewise. * src/locking/lock_manager.c (virLockManagerPluginNew) [!HAVE_DLFCN_H]: Likewise. * src/locking/lock_driver_nop.c (virLockManagerNopNew) (virLockManagerNopAddResource, virLockManagerNopAcquire) (virLockManagerNopRelease, virLockManagerNopInquire): Likewise. --- cfg.mk | 21 - src/fdstream.c | 28 ++-- src/fdstream.h | 6 +++--- src/locking/lock_driver_nop.c | 10 +- src/locking/lock_manager.c | 7 --- src/util/command.c | 2 +- src/util/iohelper.c | 18 +- src/util/util.c | 14 +++--- 8 files changed, 63 insertions(+), 43 deletions(-) @@ -516,13 +516,13 @@ virFDStreamOpenFileInternal(virStreamPtr st, int errfd = -1; pid_t pid = 0; - VIR_DEBUG(st=%p path=%s flags=%x offset=%llu length=%llu mode=%o delete=%d, - st, path, flags, offset, length, mode, delete); + VIR_DEBUG(st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o delete=%d, + st, path, oflags, offset, length, mode, delete); In 02/27 you added a syntax-check rule to enforce flags=%x. Does this automatically cover oflags too, or does this need a change in the rule? @@ -564,7 +564,7 @@ virFDStreamOpenFileInternal(virStreamPtr st, cmd = virCommandNewArgList(LIBEXECDIR /libvirt_iohelper, path, NULL); - virCommandAddArgFormat(cmd, %d, flags); + virCommandAddArgFormat(cmd, %d, oflags); In 02/27 you changed the printing of flags from %d to %x in debug output. Maybe we should do that here too for consistence and adapt libvirt_iohelper? Or is there any possibility for a version mismatch between libvirt and libvirt_iohelper and we cannot change this anymore without breaking backwards compatibility? virCommandAddArgFormat(cmd, %d, mode); Same comment applies here about mode and switching from %d to %o. ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] command: avoid fd leak on failure
On 07/12/2011 08:43 PM, Daniel Veillard wrote: On Tue, Jul 12, 2011 at 02:09:34PM -0600, Eric Blake wrote: virCommandTransferFD promises that the fd is no longer owned by the caller. Normally, we want the fd to remain open until the child runs, but in error situations, we must close it earlier. * src/util/command.c (virCommandTransferFD): Close fd now if we can't track it to close later. --- @@ -749,7 +749,13 @@ virCommandPreserveFD(virCommandPtr cmd, int fd) void virCommandTransferFD(virCommandPtr cmd, int fd) { -return virCommandKeepFD(cmd, fd, true); +virCommandKeepFD(cmd, fd, true); +if ((!cmd || cmd-has_error) fd STDERR_FILENO) { +/* We must close the fd now, instead of waiting for + * virCommandRun, if there was an error that prevented us from + * adding the fd to cmd. */ +VIR_FORCE_CLOSE(fd); +} } Hum, it's a bit like memory allocation, it's better to have the one who allocates it to free it to avoid double frees. Maybe we could instead raise and error in the caller chain, and have it freed at teh right place (unless it really get messy). That was the whole reason we introduced virCommandPreserveFD vs. virCommandTransferFD. With preserve, the caller both opens and closes fd. With transfer, the caller opens fd, then transfers it to virCommand and must not touch it again; virCommand then guarantees that it will be closed after the child runs. The problem was that we were not closing the fd in the few cases where either the child could not be run (due to a previous error, or because the fd was out of range of fdset). But I'm open to the idea of changing the signature: virCommandPreserveFD(virCommandPtr, int) - remains as-is virCommandTransferFD(virCommandPtr, int *) - change to passing the address of an fd, so that fd in the caller is set to -1 as part of the transfer process I'll post a v2 along those lines, so you can compare the difference (and also so that you'll see that existing callers are already abandoning fd after calling transfer, so setting it to -1 is only a safety valve). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 28/27] build: avoid ATTRIBUTE_UNUSED in headers
2011/7/9 Eric Blake ebl...@redhat.com: The compiler might optimize based on our declaration that something is unused. Can this actually happen? The unused marker only says that something _might_ be unused. I don't think that a compiler can optimize something based on this when it cannot actually prove that it is really unused. Putting that declaration in the header risks getting out of sync with the actual implementation, so it belongs better only in the .c files. We were mostly compliant, and a new syntax check will help us in the future. This is a valid point. * cfg.mk (sc_avoic_attribute_unused_in_header): New syntax check. * src/nodeinfo.h (nodeGetCPUStats, nodeGetMemoryStats): Delete attribute already present in .c file. * src/qemu/qemu_domain.h (qemuDomainEventFlush): Likewise. * src/util/virterror_internal.h (virReportErrorHelper): Parameters are actually used by .c file. * src/xenxs/xen_sxpr.h (xenFormatSxprDisk): Adjust prototype. * src/xenxs/xen_sxpr.c (xenFormatSxprDisk): Delete unused argument. (xenFormatSxpr): Adjust caller. * src/xen/xend_internal.c (xenDaemonAttachDeviceFlags) (xenDaemonUpdateDeviceFlags): Likewise. Suggested by Daniel Veillard. --- As suggested here: https://www.redhat.com/archives/libvir-list/2011-July/msg00501.html cfg.mk | 8 src/nodeinfo.h | 4 ++-- src/qemu/qemu_domain.h | 2 +- src/util/virterror_internal.h | 8 src/xen/xend_internal.c | 12 +--- src/xenxs/xen_sxpr.c | 5 ++--- src/xenxs/xen_sxpr.h | 3 +-- 7 files changed, 23 insertions(+), 19 deletions(-) ACK. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Save domain status ASAP after creating qemu process
When creating new qemu process we saved domain status XML only after the process was fully setup and running. In case libvirtd was killed before the whole process finished, once libvirtd started again it didn't know anything about the new process and we end up with an orphaned qemu process. Let's save the domain status XML as soon as we know the PID so that libvirtd can kill the process on restart. --- src/qemu/qemu_process.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 52a73b8..d0085e0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2226,6 +2226,10 @@ qemuProcessUpdateState(struct qemud_driver *driver, virDomainObjPtr vm) VIR_DEBUG(Domain %s was paused while its monitor was disconnected; changing state to paused, vm-def-name); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_UNKNOWN); +} else if (state == VIR_DOMAIN_SHUTOFF running) { +VIR_DEBUG(Domain %s finished booting; changing state to running, + vm-def-name); +virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); } return 0; @@ -2349,6 +2353,12 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa if (qemuProcessUpdateState(driver, obj) 0) goto error; +if (virDomainObjGetState(obj, NULL) == VIR_DOMAIN_SHUTOFF) { +VIR_DEBUG(Domain '%s' wasn't fully started yet, killing it, + obj-def-name); +goto error; +} + /* If upgrading from old libvirtd we won't have found any * caps in the domain status, so re-query them */ @@ -2463,6 +2473,7 @@ int qemuProcessStart(virConnectPtr conn, vm-def-id = driver-nextvmid++; priv-fakeReboot = false; +virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); /* Run an early hook to set-up missing devices */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { @@ -2719,6 +2730,12 @@ int qemuProcessStart(virConnectPtr conn, #endif } +VIR_DEBUG(Writing early domain status to disk); +if (virDomainSaveStatus(driver-caps, driver-stateDir, vm) 0) { +ret = -1; +goto cleanup; +} + VIR_DEBUG(Waiting for handshake from child); if (virCommandHandshakeWait(cmd) 0) { ret = -1; -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 02/27] maint: print flags in hex during debug
On 07/13/2011 03:49 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: Continuation of commit 313ac7fd, and enforce things with a syntax check. Technically, virNetServerClientCalculateHandleMode is not printing a mode_t, but rather a collection of VIR_EVENT_HANDLE_* bits; however, these bits are 8, so there is no different in the output, and that was the easiest way to silence the new syntax check. flags_debug src/libvirt-qemu.c:117:VIR_DEBUG(conn=%p, pid=%u, flags=%u, conn, pid, flags); maint.mk: debug flag values with %x Dan commited this after you posted your series. So ACK with that new offender fix too. Yep, I noticed that while rebasing as well. Fixed and pushed, along with patch 1/27. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: Avoid duplicating virFileOpenAsNoFork in virFileOpenAs
In 2f4d2496a88055a8343b3efca618522da8715d92 I didn't notice that one part of virFileOpenAs doesn't actually call to virFileOpenAsNoFork but rather includes a copy of the code from there. --- src/util/util.c | 35 +-- 1 files changed, 5 insertions(+), 30 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 62e0152..0afb7a2 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -754,7 +754,6 @@ int virFileOpenAs(const char *path, int openflags, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) { -struct stat st; pid_t pid; int waitret, status, ret = 0; int fd = -1; @@ -821,6 +820,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, /* fall back to the simpler method, which works better in * some cases */ VIR_FORCE_CLOSE(fd); +flags = ~VIR_FILE_OPEN_AS_UID; return virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags); } if (!ret) @@ -845,36 +845,11 @@ parenterror: ret = -errno; goto childerror; } -if ((fd = open(path, openflags, mode)) 0) { -ret = -errno; -if (ret != -EACCES) { -/* in case of EACCES, the parent will retry */ -virReportSystemError(errno, - _(child failed to create file '%s'), - path); -} -goto childerror; -} -if (fstat(fd, st) == -1) { -ret = -errno; -virReportSystemError(errno, _(stat of '%s' failed), path); -goto childerror; -} -if ((st.st_gid != gid) - (fchown(fd, -1, gid) 0)) { -ret = -errno; -virReportSystemError(errno, _(cannot chown '%s' to (%u, %u)), - path, (unsigned int) uid, (unsigned int) gid); -goto childerror; -} -if ((flags VIR_FILE_OPEN_FORCE_PERMS) - (fchmod(fd, mode) 0)) { -ret = -errno; -virReportSystemError(errno, - _(cannot set mode of '%s' to %04o), - path, mode); + +ret = virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags); +if (ret 0) goto childerror; -} +fd = ret; do { ret = sendfd(pair[1], fd); -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC on libvirt-qpid naming
The name libvirt-qpid is probably not as descriptive of the project as it could be. If we want to change the name then now is our best opportunity to do so, as part of the process to convert it to QMFv2 and make it a matahari agent - potentially breaking backward compatibility with existing consoles. As this project is still remaining outside of the matahari project, how do folks feel about changing the name to libvirt-qmf? regards, Zane. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Save domain status ASAP after creating qemu process
On 07/13/2011 07:58 AM, Jiri Denemark wrote: When creating new qemu process we saved domain status XML only after the process was fully setup and running. In case libvirtd was killed before the whole process finished, once libvirtd started again it didn't know anything about the new process and we end up with an orphaned qemu process. Let's save the domain status XML as soon as we know the PID so that libvirtd can kill the process on restart. --- src/qemu/qemu_process.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Avoid duplicating virFileOpenAsNoFork in virFileOpenAs
On 07/13/2011 08:08 AM, Jiri Denemark wrote: In 2f4d2496a88055a8343b3efca618522da8715d92 I didn't notice that one part of virFileOpenAs doesn't actually call to virFileOpenAsNoFork but rather includes a copy of the code from there. --- src/util/util.c | 35 +-- 1 files changed, 5 insertions(+), 30 deletions(-) That duplication has been around longer than your change to avoid fchown (in fact, it was around even before I started tweaking things in commit 055d4ff8 to use SCM_RIGHTS, back when it was named virFileOperationNoFork), but this is indeed a nice reduction in code size. ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix a regression of attaching device
qemuDomainModifyDeviceFlags: Changing ret to 0, but don't reset it to -1 on failure, it's not good idea to change the value of ret in the codes to do some condition checking. This patch fix it. --- src/qemu/qemu_driver.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a6b48e..ae11c68 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4892,10 +4892,10 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, _(unknown domain modify action %d), action); break; } -} else -ret = 0; +} -if (!ret (flags VIR_DOMAIN_AFFECT_LIVE)) { +if (!(flags VIR_DOMAIN_AFFECT_CONFIG) +(flags VIR_DOMAIN_AFFECT_LIVE)) { /* If dev exists it was created to modify the domain config. Free it. */ virDomainDeviceDefFree(dev); dev = virDomainDeviceDefParse(driver-caps, vm-def, xml, -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 04/27] libvirt-qemu: use unsigned flags
On 07/13/2011 03:54 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: Like commit 1740c381, but for libvirt-qemu. * src/remote/qemu_protocol.x (qemu_monitor_command_args): Adjust type to match API. * src/qemu_protocol-structs: Update accordingly. --- v2: new patch src/qemu_protocol-structs |2 +- src/remote/qemu_protocol.x |4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) Should we have a syntax-check rule to enforce unsigned int flags in the RPC protocol like you added a rule to do this in the public API. The syntax-check rule in 27/27 should cover this - it will complain if we introduce a 'int flags;' in any of the *protocol.x files, which should prod us to use 'unsigned int flags;' instead. Or are you referring to the fact that patch 27/27 only checked libvirt.h.in for additions of '...long flags', to ensure that no more than the existing 4 uses for migration are added? In that case, yeah, that rule can be enhanced to check include/libvirt/libvirt-qemu.h as well, to ensure that all new public APIs, whether for libvirt proper or for libvirt-qemu, use 'unsigned int flags'. But that's a tweak to 27, not to this one, so I pushed 4/27 as-is. ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix a regression of attaching device
On 07/13/2011 09:17 AM, Osier Yang wrote: qemuDomainModifyDeviceFlags: Changing ret to 0, but don't reset it to -1 on failure, it's not good idea to change the value of ret in the codes to do some condition checking. This patch fix it. Can you identify which commit introduced the regression, and what behavior actually broke as a result? The commit message will be more useful with that information. --- src/qemu/qemu_driver.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a6b48e..ae11c68 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4892,10 +4892,10 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, _(unknown domain modify action %d), action); break; } -} else -ret = 0; +} -if (!ret (flags VIR_DOMAIN_AFFECT_LIVE)) { +if (!(flags VIR_DOMAIN_AFFECT_CONFIG) +(flags VIR_DOMAIN_AFFECT_LIVE)) { I'm not sure I agree with this change. The logic flow before this patch is: if config make the change in a copy, or record that the change failed if live, and either no config or change to config copy is happy make the change live, or record that change failed if config, and either no live or change to live is happy commit the changed copy made earlier Your proposed change makes it so that we can now change live even if we are going to fail to change the config, which is not a good idea. Why not instead do: if config { make change in a copy if ret 0 goto error } if live { make change in live if ret 0 goto error } if config commit the changed copy made earlier -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 27/27] build: add syntax check for proper flags use
On 07/08/2011 01:26 PM, Eric Blake wrote: Enforce the recent flags cleanups - we want to use 'unsigned int flags' in any of our APIs (except where backwards compatibility is important, in the public migration APIs), and that all flags are checked for validity (except when there are stub functions that completely ignore the flags argument). There are a few minor tweaks done here to avoid false positives: signed arguments passed to open() are renamed oflags, and flags arguments that are legitimately ignored are renamed flags_unused. * cfg.mk (sc_flags_usage): New rule. (exclude_file_name_regexp--sc_flags_usage): And a few exemptions. +sc_flags_usage: + @test $$(grep -c 'long flags' \ + $(srcdir)/include/libvirt/libvirt.h.in) != 4 \ + { echo '$(ME): new API should use unsigned int flags' 12; \ + exit 1; } || : As mentioned in 4/27, this doesn't cover all public APIs. I'm thinking of squashing this in (I'll post a full v2 once I actually rebase it on top of all ACK'd posts, but will start the review now): diff --git i/cfg.mk w/cfg.mk index 4ab5752..2e177ff 100644 --- i/cfg.mk +++ w/cfg.mk @@ -280,14 +280,17 @@ sc_flags_debug: # Prefer 'unsigned int flags', along with checks for unknown flags. # For historical reasons, we are stuck with 'unsigned long flags' in -# migration and in a few other places. -# Three tests in this check: a fixed number of non-int flags in public -# API, no flags marked unused, and 'unsigned' should appear before any -# declaration of a flags variable (hence the prohibit line of [^d]). -# The existence of long long makes the third test slightly harder. +# migration, so check for those known 4 instances and no more in public +# API. Also check that no flags are marked unused, and 'unsigned' should +# appear before any declaration of a flags variable (acheived by +# prohibiting the word prior to the type from ending in anything other +# than d). The existence of long long, and of documentation about +# flags, makes the regex in the third test slightly harder. sc_flags_usage: - @test $$(grep -c 'long flags' \ - $(srcdir)/include/libvirt/libvirt.h.in) != 4 \ + @test $$(cat $(srcdir)/include/libvirt/libvirt.h.in\ + $(srcdir)/include/libvirt/virterror.h \ + $(srcdir)/include/libvirt/libvirt-qemu.h\ + | grep -c '\(long\|unsigned\) flags') != 4 \ { echo '$(ME): new API should use unsigned int flags' 12; \ exit 1; } || : @prohibit=' flags ''ATTRIBUTE_UNUSED' \ -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 27/27] build: add syntax check for proper flags use
On 07/13/2011 07:35 AM, Matthias Bolte wrote: @@ -516,13 +516,13 @@ virFDStreamOpenFileInternal(virStreamPtr st, int errfd = -1; pid_t pid = 0; -VIR_DEBUG(st=%p path=%s flags=%x offset=%llu length=%llu mode=%o delete=%d, - st, path, flags, offset, length, mode, delete); +VIR_DEBUG(st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o delete=%d, + st, path, oflags, offset, length, mode, delete); In 02/27 you added a syntax-check rule to enforce flags=%x. Does this automatically cover oflags too, or does this need a change in the rule? Hmm, as committed in 2/27, it checked '\flags=%...'. If we remove the \ then it would also cover oflags, and that's probably a good change to make. I'll do it, and see what fallout it causes. @@ -564,7 +564,7 @@ virFDStreamOpenFileInternal(virStreamPtr st, cmd = virCommandNewArgList(LIBEXECDIR /libvirt_iohelper, path, NULL); -virCommandAddArgFormat(cmd, %d, flags); +virCommandAddArgFormat(cmd, %d, oflags); In 02/27 you changed the printing of flags from %d to %x in debug output. Maybe we should do that here too for consistence and adapt libvirt_iohelper? Or is there any possibility for a version mismatch between libvirt and libvirt_iohelper and we cannot change this anymore without breaking backwards compatibility? I thought about this (independently, because I'm working on an O_DIRECT patch for iohelper), and my conclusion was that normally libvirtd and libvirt_iohelper are installed at the same time. However, there is indeed a window where: Older libvirtd is running, and you install the newer executables. Then the older libvirtd calls out to libvirt_iohelper, and executes the new one. That is, if you install a new libvirtd package, but don't restart the libvirtd daemon, then the new libvirt_iohelper must understand the syntax that will be handed it by the older still-running libvirtd. Therefore, my conclusion is that we can't change any existing command lines, but can only add new ones. Adding new could include using 0x or 0 prefixes (the old code output decimal, so it is distinguishable, and the new iohelper command line would then be that much easier to understand if you browse /proc/nnn/ to learn the command line that iohelper is using). virCommandAddArgFormat(cmd, %d, mode); Same comment applies here about mode and switching from %d to %o. I'll look into that, as a separate patch. ACK. I already have to post a v2, so we'll get another round of review on this commit as well as the couple of fallout suggestions raised here. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't overwrite errors by closefd in error paths
On 07/13/2011 03:25 AM, Jiri Denemark wrote: When qemuMonitorCloseFileHandle is called in error path, we need to preserve the original error since a possible further error when running closefd monitor command is not very useful to users. --- src/qemu/qemu_monitor.c | 34 +- src/qemu/qemu_monitor.h |3 ++- 2 files changed, 27 insertions(+), 10 deletions(-) @@ -1962,22 +1962,34 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon, int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, - const char *fdname) + const char *fdname, + bool preserveError) Why the bool argument? Every one of the callers was adjusted to set it to true, so if no one sets it to false, it seems like it makes more sense to blindly preserveError instead of make it parameterizable, without having to tweak any callers. { -int ret; +int ret = -1; +virErrorPtr error = NULL; + VIR_DEBUG(mon=%p fdname=%s, mon, fdname); +if (preserveError) +error = virSaveLastError(); + if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, %s, _(monitor must not be NULL)); -return -1; +goto cleanup; } if (mon-json) ret = qemuMonitorJSONCloseFileHandle(mon, fdname); else ret = qemuMonitorTextCloseFileHandle(mon, fdname); + +cleanup: +if (error) { +virSetError(error); +virFreeError(error); +} return ret; } ACK to the concept, though. If you have a future patch that will pass false, then this looks okay, otherwise, it's probably worth a simpler v2. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: Avoid memory leak on metadata fetching
Getting metadata on storage allocates a memory (path) which need to be freed after use otherwise it gets leaked. This means after use of virStorageFileGetMetadataFromFD or virStorageFileGetMetadata one must call virStorageFileFreeMetadata to free it. Right now this function frees only one variable in metadata structure, but is prepared to be extended in the future. --- src/conf/domain_conf.c |8 +++- src/libvirt_private.syms |1 + src/qemu/qemu_driver.c |3 +++ src/storage/storage_backend_fs.c |5 +++-- src/util/storage_file.c | 15 +++ src/util/storage_file.h |2 ++ 6 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 39e59b9..2ebd012 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11055,6 +11055,9 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, int ret = -1; size_t depth = 0; char *nextpath = NULL; +virStorageFileMetadata meta; + +memset(meta, 0, sizeof(virStorageFileMetadata)); if (!disk-src || disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; @@ -11084,7 +11087,6 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, paths = virHashCreate(5, NULL); do { -virStorageFileMetadata meta; const char *path = nextpath ? nextpath : disk-src; int fd; @@ -11127,6 +11129,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, depth++; nextpath = meta.backingStore; +meta.backingStore = NULL; /* Stop iterating if we reach a non-file backing store */ if (nextpath !meta.backingStoreIsFile) { @@ -11137,6 +11140,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, format = meta.backingStoreFormat; +virStorageFileFreeMetadata(meta); + if (format == VIR_STORAGE_FILE_AUTO !allowProbing) format = VIR_STORAGE_FILE_RAW; /* Stops further recursion */ @@ -11151,6 +11156,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, cleanup: virHashFree(paths); VIR_FREE(nextpath); +virStorageFileFreeMetadata(meta); return ret; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3237d18..e06c7fc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -940,6 +940,7 @@ virStorageGenerateQcowPassphrase; # storage_file.h virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; +virStorageFileFreeMetadata; virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; virStorageFileIsSharedFS; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a6b48e..54f9bba 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6358,6 +6358,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, virCheckFlags(0, -1); +memset(meta, 0, sizeof(virStorageFileMetadata)); + qemuDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); qemuDriverUnlock(driver); @@ -6511,6 +6513,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, cleanup: VIR_FORCE_CLOSE(fd); +virStorageFileFreeMetadata(meta); if (vm) virDomainObjUnlock(vm); return ret; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d87401f..02b1637 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -118,7 +118,6 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, ret = 0; } } else { -VIR_FREE(meta.backingStore); ret = 0; } @@ -147,10 +146,12 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, */ } +virStorageFileFreeMetadata(meta); + return ret; cleanup: -VIR_FREE(*backingStore); +virStorageFileFreeMetadata(meta); return -1; } diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 06cabc8..19c2464 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -819,6 +819,8 @@ virStorageFileProbeFormat(const char *path) * it indicates the image didn't specify an explicit format for its * backing store. Callers are advised against probing for the * backing store format in this case. + * + * Caller must free @meta after use via virStorageFileFreeMetadata. */ int virStorageFileGetMetadataFromFD(const char *path, @@ -892,6 +894,8 @@ cleanup: * it indicates the image didn't specify an explicit format for its * backing store. Callers are advised against probing for the * backing store format in this case. + * + * Caller MUST free @meta after use via virStorageFileFreeMetadata. */ int virStorageFileGetMetadata(const char *path, @@ -912,6 +916,17 @@ virStorageFileGetMetadata(const char *path, return ret; } +/** + * virStorageFileFreeMetadata: + * + * Free pointers in passed structure, but not memory
Re: [libvirt] [PATCHv2 07/27] storage: reject unknown flags
On 07/13/2011 05:34 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: * src/storage/storage_backend.c (virStorageBackendCreateBlockFrom) (virStorageBackendCreateQemuImg) (virStorageBackendCreateQcowCreate): Reject unknown flags. ACK. I've pushed 5, 6, and 7. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] qemu: build failed due to unused variables
On 07/13/2011 06:44 AM, Osier Yang wrote: 于 2011年07月13日 18:11, Peter Krempa 写道: While compiling on F15 build crashed (probably because of new GCC). --- AUTHORS|1 + src/qemu/qemu_driver.c |7 --- 2 files changed, 5 insertions(+), 3 deletions(-) I meet the same problem on FC15, ACK Pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix missing prompt message for 'snapshot-delete' command
On 07/13/2011 06:51 AM, Osier Yang wrote: 于 2011年07月13日 16:36, Nan Zhang 写道: Make the command 'virsh snapshot-delete' has the appropriate prompt message when executing sucessful or failed. -if (virDomainSnapshotDelete(snapshot, flags) 0) You can keep this line unchanged, and switch the prompt strings for failure and success. But it's fine, ACK +if (virDomainSnapshotDelete(snapshot, flags) == 0) { +vshPrint(ctl, _(Domain snapshot %s deleted\n), name); +} else { +vshError(ctl, _(Failed to delete snapshot %s), name); goto cleanup; +} I didn't see any reason to reorder things. I added Nan to AUTHORS and pushed; let me know if I need to adjust any spellings. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] Small fixes on rpc generator scripts
On 07/13/2011 04:19 AM, Osier Yang wrote: 1. Fix typos caused by the renaming 2. It should substitute remote/remote_protocol.h to remote_protocol.h. --- src/rpc/gendispatch.pl |6 +++--- src/rpc/genprotocol.pl |2 +- 2 files changed, 4 insertions(+), 4 deletions(-) ACK. +++ b/src/rpc/genprotocol.pl @@ -59,7 +59,7 @@ while (RPCGEN) { s/xdr_u_quad_t/xdr_uint64_t/g; s/xdr_quad_t/xdr_int64_t/g; s/(?!IXDR_GET_INT32 )IXDR_GET_LONG/IXDR_GET_INT32/g; -s,#include \./remote/remote_protocol\.h,#include remote_protocol.h,; +s,#include remote/remote_protocol\.h,#include remote_protocol.h,; I wonder if we could even go one step further in using: s,#include .*remote/remote_protocol\.h,#include remote_protocol.h,; but as written, this was okay, and things compiled fine both before and after this change. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 08/27] esx: reject unknown flags
On 07/13/2011 06:15 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: Silently ignored flags get in the way of new features that use those flags. @@ -3829,12 +3837,14 @@ esxDomainMigratePrepare(virConnectPtr dconn, int *cookielen ATTRIBUTE_UNUSED, const char *uri_in ATTRIBUTE_UNUSED, char **uri_out, -unsigned long flags ATTRIBUTE_UNUSED, +unsigned long flags, const char *dname ATTRIBUTE_UNUSED, unsigned long resource ATTRIBUTE_UNUSED) { esxPrivate *priv = dconn-privateData; +virCheckFlags(0, -1); + Actually this is wrong. This was implemented before libvirt knew about VIR_MIGRATE_PERSIST_DEST VIR_MIGRATE_UNDEFINE_SOURCE But an ESX migration has exactly the semantic implied by those two flags. So actually those two flags have to be passed always. I should have fixed that ages ago. So, do we fix that now, or do we push this patch as-is (with the semantic change of rejecting the flags that are currently useful) along with your cleanup as a followup (with requiring the two flags)? I'm leaning a bit towards the latter (fix the flags to require the bits that must be present), so I'll post a v2. Also if the domain is running ESX does a live migration always, so VIR_MIGRATE_LIVE has to passed too. ESX supports migration of inactive domains. It seems that migration in libvirt is only meant for active domains, but this isn't documented well. Right now, qemu domains only get migrated if the domain is active, but I could totally see enhancing that to support migration of persistent but inactive domains (and it's a lot simpler - dumpxml on the source, and define on the destination, without having to do any handshaking between qemu processes). In fact, I'd love that as a feature addition! -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/8] UndefineFlags: Define the public API
On 07/13/2011 04:19 AM, Osier Yang wrote: Only support to remove domain managed state file when undefining s/to remove/removing/ the domain currently. --- include/libvirt/libvirt.h.in | 10 ++ python/generator.py |1 + src/libvirt_public.syms |5 + 3 files changed, 16 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index d5a7105..98f1454 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1200,6 +1200,16 @@ int virDomainMemoryPeek (virDomainPtr dom, virDomainPtrvirDomainDefineXML (virConnectPtr conn, const char *xml); int virDomainUndefine (virDomainPtr domain); + +/* Domain undefine flags */ +typedef enum { +VIR_DOMAIN_UNDEFINE_MANAGED_STATE = 1, + +/* Future undefine control flags should come here */ +} virDomainUndefineFlags; + +int virDomainUndefineWithFlags (virDomainPtr domain, + unsigned int flags); I'm not sure I like the WithFlags name. We _had_ to use it on virDomainCreateWithFlags, because the public name virDomainCreateFlags was already in existence. But here, you could just as easily go with: typedef enum { ... } virDomainUndefineFlagValues; int virDomainUndefineFlags(virDomainPtr domain, unsigned int flags); and avoid the extra With. +++ b/python/generator.py @@ -366,6 +366,7 @@ skip_impl = ( 'virDomainSendKey', 'virNodeGetCPUStats', 'virNodeGetMemoryStats', +'virDomainUndefineWithFlags', Why does this have to be skipped? virDomainUndefine is not skipped, and it seems like having python bindings for this new function would be worthwhile. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] UndefineFlags: Implement the public API
On 07/13/2011 04:19 AM, Osier Yang wrote: --- src/libvirt.c | 53 - 1 files changed, 52 insertions(+), 1 deletions(-) per my comments in 2/8, /** + * virDomainUndefineWithFlags: s/virDomainUndefineWithFlags/virDomainUndefineFlags/ + * @domain: pointer to a defined domain + * @flags: bitwise-or of supported virDomainUndefineFlags s/virDomainUndefineFlags/virDomainUndefineFlagValues/ + * + * Undefine a domain but does not stop it if it is running Copy and paste from the existing virDomainUndefine, but I think it would read better (in both places) as: Undefine a persistent domain. A running domain is left running but converted into a transient domain. + * + * If VIR_DOMAIN_UNDEFINE_MANAGED_STATE is specified, the domain s/the/any/ - since a managed state file might not exist + * managed state file will be removed along with domain undefine + * process, the entire domain undefine process will fail if s/process, the/process, and the/ + * it fails on removing the managed state file. We also need to clearly document what happens if a managed state file exists but the flag is not present. My preference would be rewording this entire paragraph as: If the domain has a managed state file (see virDomainHasManagedSaveImage()), then including VIR_DOMAIN_UNDEFINE_MANAGED_STATE in @flags will also remove that file, and omitting the flag will cause the undefine process to fail. and back at virDomainUndefine, add a paragraph: If the domain has a managed state file (see virDomainHasManagedSaveImage()), then the undefine will fail. See virDomainUndefineFlags() for more control. The code additions look fine except for the choice of API name, but the documentation is just as important, so this needs a v2. Also, I'd probably squash patch 2 and 3 into one patch - that is, it would be nice to add both the declaration and the documentation of the API in the same patch. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] UndefineFlags: Define the internal API
On 07/13/2011 04:19 AM, Osier Yang wrote: --- src/driver.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index 71a52c9..3db9fb7 100644 --- a/src/driver.h +++ b/src/driver.h @@ -219,6 +219,9 @@ typedef virDomainPtr typedef int (*virDrvDomainUndefine) (virDomainPtr dom); typedef int +(*virDrvDomainUndefineWithFlags) (virDomainPtr dom, + unsigned int flags); This needs to reflect whatever naming we have in 2/8. Conditional ACK once we agree on the API name. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/8] UndefineFlags: Implement qemu driver handling
On 07/13/2011 04:19 AM, Osier Yang wrote: --- src/qemu/qemu_driver.c | 31 +++ 1 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f962e2c..a9f6986 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2402,10 +2402,8 @@ cleanup: static char * qemuDomainManagedSavePath(struct qemud_driver *driver, virDomainObjPtr vm) { char *ret; -char uuidstr[VIR_UUID_STRING_BUFLEN]; -virUUIDFormat(vm-def-uuid, uuidstr); -if (virAsprintf(ret, %s/%s.save, driver-saveDir, uuidstr) 0) { +if (virAsprintf(ret, %s/%s.save, driver-saveDir, vm-def-name) 0) { This hunk won't apply to existing libvirt.git. Did you forget to rebase out your first attempt? @@ -4312,6 +4315,20 @@ static int qemudDomainUndefine(virDomainPtr dom) { goto cleanup; } +if (flags VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { +name = qemuDomainManagedSavePath(driver, vm); + +if (name == NULL) +goto cleanup; + +if (virFileExists(name) (unlink(name) 0)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(failed on removing domain managed state +file '%s'), name); +goto cleanup; +} +} I think we need to explicitly reject attempts to undefine a domain while a state file exists. That is, I think this logic needs to be: name = qemuDomainManagedSavePath(driver, vm); if (name == NULL) goto cleanup; if (virFileExists(name)) { if (flags VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { if (unlink(name) 0) { error - failed to remove } } else { error - refusing to undefine with managed state file present } } Yes, it will change existing behavior (previously, you could undefine a domain and leave the state file behind), but that was unsafe, and this is arguably a bug fix. The default should be as safe as possible, with the flags (VIR_DOMAIN_UNDEFINE_MANAGED_STATE) in place to make things faster. @@ -8487,6 +8509,7 @@ static virDriver qemuDriver = { .domainCreateWithFlags = qemudDomainStartWithFlags, /* 0.8.2 */ .domainDefineXML = qemudDomainDefine, /* 0.2.0 */ .domainUndefine = qemudDomainUndefine, /* 0.2.0 */ +.domainUndefineWithFlags = qemudDomainUndefineWithFlags, /* 0.9.4 */ Also, I think that for every hypervisor that supports domainUndefine, we should add a (trivial) domainUndefine[With]Flags wrapper, so that the new API is available everywhere in 0.9.4, but do that as a separate patch. I'm about to post a series to add virDomainSaveFlags, which might serve as an example for what I'm thinking about. That series will add a no-semantic-change wrapper in the first commit, and only later changes the qemu wrapper to learn to honor a new flag value. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] UndefineFlags: Implement libxl driver handling
On 07/13/2011 04:19 AM, Osier Yang wrote: --- src/libxl/libxl_driver.c | 32 1 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 808480f..f04931b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -220,10 +220,8 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) static char * libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { char *ret; -char uuidstr[VIR_UUID_STRING_BUFLEN]; -virUUIDFormat(vm-def-uuid, uuidstr); -if (virAsprintf(ret, %s/%s.save, driver-saveDir, uuidstr) 0) { +if (virAsprintf(ret, %s/%s.save, driver-saveDir, vm-def-name) 0) { Same rebase problem as in 5/8. + +if (flags VIR_DOMAIN_UNDEFINE_MANAGED_STATE) { +name = libxlDomainManagedSavePath(driver, vm); + +if (name == NULL) +goto cleanup; + +if (virFileExists(name) (unlink(name) 0)) { +libxlError(VIR_ERR_INTERNAL_ERROR, + _(failed on removing domain managed state + file '%s'), name); +goto cleanup; +} +} Same logic problem as in 5/8. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] UndefineFlags: Implement the remote protocol
On 07/13/2011 04:19 AM, Osier Yang wrote: With small typo fixes (s/remote_generator/gendispatch/, and s/remote_internal/remote_driver/) --- src/remote/qemu_protocol.x |2 +- src/remote/remote_driver.c |5 +++-- src/remote/remote_protocol.x | 14 ++ src/remote_protocol-structs |4 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x index fa453f4..9381f7c 100644 --- a/src/remote/qemu_protocol.x +++ b/src/remote/qemu_protocol.x @@ -43,7 +43,7 @@ const QEMU_PROTOCOL_VERSION = 1; enum qemu_procedure { /* Each function must have a two-word comment. The first word is - * whether remote_generator.pl handles daemon, the second whether + * whether gendispatch.pl handles daemon, the second whether If you haven't already pushed 1/8, then I'd squash these typo fixes into that patch. @@ -4177,6 +4177,7 @@ static virDriver remote_driver = { .domainCreateWithFlags = remoteDomainCreateWithFlags, /* 0.8.2 */ .domainDefineXML = remoteDomainDefineXML, /* 0.3.0 */ .domainUndefine = remoteDomainUndefine, /* 0.3.0 */ +.domainUndefineWithFlags = remoteDomainUndefineWithFlags, /* 0.9.4 */ May reflect any API renaming. .domainAttachDevice = remoteDomainAttachDevice, /* 0.3.0 */ .domainAttachDeviceFlags = remoteDomainAttachDeviceFlags, /* 0.7.7 */ .domainDetachDevice = remoteDomainDetachDevice, /* 0.3.0 */ @@ -4244,7 +4245,7 @@ static virDriver remote_driver = { .domainMigratePerform3 = remoteDomainMigratePerform3, /* 0.9.2 */ .domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */ -.domainSendKey = remoteDomainSendKey, /* 0.9.3 */ +.domainSendKey = remoteDomainSendKey /* 0.9.3 */ Spurious change. Leave the trailing comma here. +++ b/src/remote/remote_protocol.x @@ -339,7 +339,7 @@ struct remote_node_get_memory_stats { * connection). Errors are returned implicitly in the RPC protocol. * * Please follow the naming convention carefully - this file is - * parsed by 'remote_generator.pl'. + * parsed by 'gendispatch.pl'. Another typo fix to squash into 1/8. @@ -2383,14 +2388,15 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_CPU_STATS = 227, /* skipgen skipgen */ REMOTE_PROC_NODE_GET_MEMORY_STATS = 228, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_GET_CONTROL_INFO = 229, /* autogen autogen */ -REMOTE_PROC_DOMAIN_GET_VCPU_PIN_INFO = 230 /* skipgen skipgen */ +REMOTE_PROC_DOMAIN_GET_VCPU_PIN_INFO = 230, /* skipgen skipgen */ +REMOTE_PROC_DOMAIN_UNDEFINE_WITH_FLAGS = 231 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? * Nice isn't it. Please keep it this way when adding more. Oops - you missed the blank line that this comment asked for. Conditional ACK once you split out the typo changes and account for any API rename. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/8] UndefineFlags: Extend virsh undefine to support new flag
On 07/13/2011 04:19 AM, Osier Yang wrote: --- tools/virsh.c | 12 +++- tools/virsh.pod |6 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3cdf043..f81e923 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1409,6 +1409,7 @@ static const vshCmdInfo info_undefine[] = { static const vshCmdOptDef opts_undefine[] = { {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name or uuid)}, +{undefine-managed-state, VSH_OT_BOOL, 0, N_(remove domain managed state file)}, Dave suggested '--remove-managed-state', but given the name of our existing virsh command: managedsave-remove I'd feel slightly better with: virsh undefine domain --managed-save [Meanwhile, we don't have any virsh command that directly exposes virDomainHasManagedSaveImage - that information should probably be added as a flag to an existing command, perhaps 'virsh list --managed-save' adding a column on whether a managed save image exists for each domain.] +int flags = 0; +int undefine_managed_state = vshCommandOptBool(cmd, undefine-managed-state); + +if (undefine_managed_state) +flags |= VIR_DOMAIN_UNDEFINE_MANAGED_STATE; + +if (!undefine_managed_state) +flags = -1; if (!vshConnectionUsability(ctl, ctl-conn)) return false; @@ -1440,7 +1449,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) VSH_BYNAME|VSH_BYUUID))) return false; -if (virDomainUndefine(dom) == 0) { +if (((flags == -1) ? virDomainUndefine(dom) : +virDomainUndefineWithFlags(dom, flags)) == 0) { Most times when we add a new flag to virsh, we can't support it without using the new API. But here, we can, and so I think we should. That is, if you virsh 0.9.4 as the client, and are talking to two different servers (one 0.9.3 and one 0.9.4), you get the following behavior for a domain that has a managed save image: server 0.9.3: virsh undefine dom - call virDomainHasManagedSaveImage - if that is true, then refuse to run virDomainUndefine (thus papering over the 0.9.3 safety bug). If false, then virDomainUndefine is safe. virsh undefine dom --managed-save - tries virDomainUndefineFlags, which fails with unimplemented. Falls back to combo virDomainManagedSaveRemove/virDomainUndefine, which does the job we want. server 0.9.4 virsh undefine dom - call virDomainHasManagedSaveImage - if that is true, then refuse to run virDomainUndefine. If false, then virDomainUndefine is safe. virsh undefine dom --managed-save - tries virDomainUndefineFlags, which succeeds and does the job we want (or fails, but with an error different than unimplemented) an alternative would be that calling 'virsh undefine dom' without flags skips virDomainHasManagedSaveImage, and just blindly calls virDomainUndefine - in that case, a server at 0.9.4 would properly fail, but a server at 0.9.3 would still expose the bug that we are trying to avoid of leaving stale managed state behind. For precedence in making virsh try a fallback to older API when the new API is unsupported, see the setvcpus command. +++ b/tools/virsh.pod @@ -804,11 +804,15 @@ hypervisor. Output the device used for the TTY console of the domain. If the information is not available the processes will provide an exit code of 1. -=item Bundefine Idomain-id +=item Bundefine Idomain-id optional I--undefine-managed-state Undefine the configuration for an inactive domain. Since it's not running the domain name or UUID must be used as the Idomain-id. Oh, this is different than libvirt.c, which claimed you can run 'undefine' on a running persistent domain. Which is it? Can undefine make a running domain transient, or must it be on an inactive domain? +If I--undefine-managed-state is specified, the managed state file will +be removed along with the domain undefine peocess, the entire domain +undefine process will fail if it fails on removing the managed state file. Given my above thoughts (and once I validate the behavior of undefine on a running domain), I'd word this as: =item Bundefine Idomain-id [I--managed-save] Undefine the configuration for a domain. If domain is not running, the domain name or UUID must be used as the Idomain-id; if the domain is running, then this converts a persistent domain to a transient domain. The I--managed-save flag guarantees that any managed state (see the Bmanagesave command) is also cleaned up. Without the flag, attempts to undefine a domain with managed state will fail. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] hyperv: Add basic driver for Microsoft Hyper-V
Domain listing, basic information retrieval and domain life cycle management is implemented. But currently the domian XML output lacks the complete devices section. The driver uses OpenWSMAN to directly communicate with an Hyper-V server over its WS-Management interface exposed via Microsoft WinRM. The driver is based on the work of Michael Sievers. This started in the same master program project group at the University of Paderborn as the ESX driver. See Michael's blog for details: http://hyperv4libvirt.wordpress.com/ --- po/POTFILES.in |1 + src/Makefile.am |1 + src/hyperv/hyperv_driver.c | 1172 ++- src/hyperv/hyperv_private.h |2 + src/hyperv/hyperv_util.c| 129 + src/hyperv/hyperv_util.h| 40 ++ 6 files changed, 1343 insertions(+), 2 deletions(-) create mode 100644 src/hyperv/hyperv_util.c create mode 100644 src/hyperv/hyperv_util.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 848d581..809ace6 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -29,6 +29,7 @@ src/esx/esx_vi_methods.c src/esx/esx_vi_types.c src/fdstream.c src/hyperv/hyperv_driver.c +src/hyperv/hyperv_util.c src/hyperv/hyperv_wmi.c src/interface/netcf_driver.c src/internal.h diff --git a/src/Makefile.am b/src/Makefile.am index 9971a90..6707181 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -388,6 +388,7 @@ HYPERV_DRIVER_SOURCES = \ hyperv/hyperv_device_monitor.c hyperv/hyperv_device_monitor.h \ hyperv/hyperv_secret_driver.c hyperv/hyperv_secret_driver.h \ hyperv/hyperv_nwfilter_driver.c hyperv/hyperv_nwfilter_driver.h \ + hyperv/hyperv_util.c hyperv/hyperv_util.h \ hyperv/hyperv_wmi.c hyperv/hyperv_wmi.h \ hyperv/hyperv_wmi_classes.c hyperv/hyperv_wmi_classes.h \ hyperv/openwsman.h diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index eb01bac..32d360b 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -39,14 +39,42 @@ #include hyperv_secret_driver.h #include hyperv_nwfilter_driver.h #include hyperv_private.h +#include hyperv_util.h +#include hyperv_wmi.h +#include openwsman.h #define VIR_FROM_THIS VIR_FROM_HYPERV +static void +hypervFreePrivate(hypervPrivate **priv) +{ +if (priv == NULL || *priv == NULL) { +return; +} + +if ((*priv)-client != NULL) { +/* FIXME: This leaks memory due to bugs in openwsman = 2.2.6 */ +wsmc_release((*priv)-client); +} + +hypervFreeParsedUri((*priv)-parsedUri); +VIR_FREE(*priv); +} + + + static virDrvOpenStatus hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { +virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; +hypervPrivate *priv = NULL; +char *username = NULL; +char *password = NULL; +virBuffer query = VIR_BUFFER_INITIALIZER; +Msvm_ComputerSystem *computerSystem = NULL; + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); /* Decline if the URI is NULL or the scheme is not hyperv */ @@ -69,28 +97,1165 @@ hypervOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) return VIR_DRV_OPEN_ERROR; } -return VIR_DRV_OPEN_SUCCESS; +/* Allocate per-connection private data */ +if (VIR_ALLOC(priv) 0) { +virReportOOMError(); +goto cleanup; +} + +if (hypervParseUri(priv-parsedUri, conn-uri) 0) { +goto cleanup; +} + +/* Set the port dependent on the transport protocol if no port is + * specified. This allows us to rely on the port parameter being + * correctly set when building URIs later on, without the need to + * distinguish between the situations port == 0 and port != 0 */ +if (conn-uri-port == 0) { +if (STRCASEEQ(priv-parsedUri-transport, https)) { +conn-uri-port = 5986; +} else { +conn-uri-port = 5985; +} +} + +/* Request credentials */ +if (conn-uri-user != NULL) { +username = strdup(conn-uri-user); + +if (username == NULL) { +virReportOOMError(); +goto cleanup; +} +} else { +username = virRequestUsername(auth, administrator, conn-uri-server); + +if (username == NULL) { +HYPERV_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Username request failed)); +goto cleanup; +} +} + +password = virRequestPassword(auth, username, conn-uri-server); + +if (password == NULL) { +HYPERV_ERROR(VIR_ERR_AUTH_FAILED, %s, _(Password request failed)); +goto cleanup; +} + +/* Initialize the openwsman connection */ +priv-client = wsmc_create(conn-uri-server, conn-uri-port, /wsman, +
[libvirt] [PATCH 0/5] Add basic driver for Microsoft Hyper-V (series version)
This is the series version of this patch https://www.redhat.com/archives/libvir-list/2011-July/msg00668.html Daniel suggested to split it for easier review. This series includes some small fixes for problem I noticed while splitting the original patch. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] hyperv: Add configure check for OpenWSMAN
--- configure.ac | 38 ++ 1 files changed, 38 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index e9d5be4..d7ebe79 100644 --- a/configure.ac +++ b/configure.ac @@ -66,6 +66,7 @@ XMLRPC_REQUIRED=1.14.0 HAL_REQUIRED=0.5.0 DEVMAPPER_REQUIRED=1.0.0 LIBCURL_REQUIRED=7.18.0 +OPENWSMAN_REQUIRED=2.2.6 LIBPCAP_REQUIRED=1.0.0 LIBNL_REQUIRED=1.1 LIBSSH2_REQUIRED=1.0 @@ -275,6 +276,8 @@ AC_ARG_WITH([lxc], AC_HELP_STRING([--with-lxc], [add Linux Container support @:@default=check@:@]),[],[with_lxc=check]) AC_ARG_WITH([esx], AC_HELP_STRING([--with-esx], [add ESX support @:@default=check@:@]),[],[with_esx=check]) +AC_ARG_WITH([hyperv], + AC_HELP_STRING([--with-hyperv], [add Hyper-V support @:@default=check@:@]),[],[with_hyperv=check]) AC_ARG_WITH([test], AC_HELP_STRING([--with-test], [add test driver support @:@default=yes@:@]),[],[with_test=yes]) AC_ARG_WITH([remote], @@ -1912,6 +1915,35 @@ LIBCURL_CFLAGS=-DCURL_DISABLE_TYPECHECK $LIBCURL_CFLAGS AC_SUBST([LIBCURL_CFLAGS]) AC_SUBST([LIBCURL_LIBS]) + +dnl +dnl check for openwsman (Hyper-V) +dnl + +OPENWSMAN_CFLAGS= +OPENWSMAN_LIBS= + +if test $with_hyperv = yes || test $with_hyperv = check; then +PKG_CHECK_MODULES(OPENWSMAN, openwsman = $OPENWSMAN_REQUIRED, [ +if test $with_hyperv = check; then +with_hyperv=yes +fi +], [ +if test $with_hyperv = check; then +with_hyperv=no +AC_MSG_NOTICE([openwsman is required for the Hyper-V driver, disabling it]) +elif test $with_hyperv = yes; then +AC_MSG_ERROR([openwsman = $OPENWSMAN_REQUIRED is required for the Hyper-V driver]) +fi +]) +fi + +if test $with_hyperv = yes ; then +AC_DEFINE_UNQUOTED([WITH_HYPERV], 1, [whether Hyper-V driver is enabled]) +fi +AM_CONDITIONAL([WITH_HYPERV], [test $with_hyperv = yes]) + + dnl dnl check for python dnl @@ -2414,6 +2446,7 @@ AC_MSG_NOTICE([xenlight: $with_libxl]) AC_MSG_NOTICE([ LXC: $with_lxc]) AC_MSG_NOTICE([PHYP: $with_phyp]) AC_MSG_NOTICE([ ESX: $with_esx]) +AC_MSG_NOTICE([ Hyper-V: $with_hyperv]) AC_MSG_NOTICE([Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) @@ -2455,6 +2488,11 @@ AC_MSG_NOTICE([ libcurl: $LIBCURL_CFLAGS $LIBCURL_LIBS]) else AC_MSG_NOTICE([ libcurl: no]) fi +if test $with_hyperv = yes ; then +AC_MSG_NOTICE([openwsman: $OPENWSMAN_CFLAGS $OPENWSMAN_LIBS]) +else +AC_MSG_NOTICE([openwsman: no]) +fi if test $with_libssh2 != no ; then AC_MSG_NOTICE([ libssh2: $LIBSSH2_CFLAGS $LIBSSH2_LIBS]) else -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] hyperv: Add basic documentation
--- docs/drivers.html.in |1 + docs/drvhyperv.html.in | 103 docs/index.html.in |3 + docs/sitemap.html.in |4 ++ src/README |3 +- 5 files changed, 113 insertions(+), 1 deletions(-) create mode 100644 docs/drvhyperv.html.in diff --git a/docs/drivers.html.in b/docs/drivers.html.in index 0428870..75038fc 100644 --- a/docs/drivers.html.in +++ b/docs/drivers.html.in @@ -28,6 +28,7 @@ listronga href=drvesx.htmlVMware ESX/a/strong/li listronga href=drvvmware.htmlVMware Workstation/Player/a/strong/li listronga href=drvxen.htmlXen/a/strong/li + listronga href=drvhyperv.htmlMicrosoft Hyper-V/a/strong/li /ul h2a name=stroageStorage drivers/a/h2 diff --git a/docs/drvhyperv.html.in b/docs/drvhyperv.html.in new file mode 100644 index 000..dec5969 --- /dev/null +++ b/docs/drvhyperv.html.in @@ -0,0 +1,103 @@ +htmlbody +h1Microsoft Hyper-V hypervisor driver/h1 +ul id=toc/ul +p +The libvirt Microsoft Hyper-V driver can manage Hyper-V 2008 R2. +/p + + +h2a name=uriConnections to the Microsoft Hyper-V driver/a/h2 +p +Some example remote connection URIs for the driver are: +/p +pre +hyperv://example-hyperv.com (over HTTPS) +hyperv://example-hyperv.com/?transport=http (over HTTP) +/pre +p +strongNote/strong: In contrast to other drivers, the Hyper-V driver +is a client-side-only driver. It connects to the Hyper-V server using +WS-Management over HTTP(S). Therefore, the +a href=remote.htmlremote transport mechanism/a provided by the +remote driver and libvirtd will not work, and you cannot use URIs like +codehyperv+ssh://example.com/code. +/p + + +h3a name=uriformatURI Format/a/h3 +p +URIs have this general form (code[...]/code marks an optional part). +/p +pre +hyperv://[username@]hostname[:port]/[?extraparameters] +/pre +p +The default HTTPS ports is 5986. If the port parameter is given, it +overrides the default port. +/p + + +h4a name=extraparamsExtra parameters/a/h4 +p +Extra parameters can be added to a URI as part of the query string +(the part following code?/code). A single parameter is formed by a +codename=value/code pair. Multiple parameters are separated by +codeamp;/code. +/p +pre +?transport=http +/pre +p +The driver understands the extra parameters shown below. +/p +table class=top_table +tr +thName/th +thValues/th +thMeaning/th +/tr +tr +td +codetransport/code +/td +td +codehttp/code or codehttps/code +/td +td +Overrides the default HTTPS transport. The default HTTP port +is 5985. +/td +/tr +/table + + +h3a name=authAuthentication/a/h3 +p +In order to perform any useful operation the driver needs to log into +the Hyper-V server. Therefore, only codevirConnectOpenAuth/code can +be used to connect to an Hyper-V server, codevirConnectOpen/code and +codevirConnectOpenReadOnly/code don't work. +To log into an Hyper-V server the driver will request credentials using +the callback passed to the codevirConnectOpenAuth/code function. +The driver passes the hostname as challenge parameter to the callback. +/p +p +strongNote/strong: Currently only codeBasic/code authentication +is supported by libvirt. This method is disabled by default on the +Hyper-V server and can be enabled via the WinRM commandline tool. +/p +pre +winrm set winrm/config/service/auth @{Basic=true} +/pre +p +To allow codeBasic/code authentication with HTTP transport WinRM +needs to allow unencrypted communication. This can be enabled via the +WinRM commandline tool. Although this is not the recommended +communication mode. +/p +pre +winrm set winrm/config/service @{AllowUnencrypted=true} +/pre + + +/body/html diff --git a/docs/index.html.in b/docs/index.html.in index 64eb84d..73c2bf8 100644 --- a/docs/index.html.in +++ b/docs/index.html.in @@ -63,6 +63,9 @@ The a href=http://www.vmware.com/;VMware Workstation and Player/a hypervisors /li li +The a href=http://www.microsoft.com/hyper-v-server/;Microsoft Hyper-V/a hypervisor + /li + li Storage on IDE/SCSI/USB disks, FibreChannel, LVM, iSCSI, NFS and filesystems /li /ul diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in index 897ee94..2c71763 100644 --- a/docs/sitemap.html.in +++ b/docs/sitemap.html.in @@ -202,6 +202,10 @@ a href=drvvmware.htmlVMware Workstation / Player/a spanDriver for VMware Workstation
Re: [libvirt] [PATCH] Add basic driver for Microsoft Hyper-V
2011/7/12 Matthias Bolte matthias.bo...@googlemail.com: Domain listing, basic information retrieval and domain life cycle management is implemented. But currently the domian XML output lacks the complete devices section. The driver uses OpenWSMAN to directly communicate with an Hyper-V server over its WS-Management interface exposed via Microsoft WinRM. Just like the ESX driver this driver includes a generator that generates the type definitions for the Hyper-V API that are feed to OpenWSMAN. Currently there are some sections in the code that are disabled because of bugs in OpenWSMAN = 2.2.6. Patches for those problems have been posted upstream. The driver is based on the work of Michael Sievers. This started in the same master program project group at the University of Paderborn as the ESX driver. See Michael's blog for details: http://hyperv4libvirt.wordpress.com/ I reposted this patch as 5 part series https://www.redhat.com/archives/libvir-list/2011-July/msg00766.html -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] hyperv: Add OpenWSMAN based client for the Hyper-V WMI API
Add a generator script to generate the structs and serialization information for OpenWSMAN. openwsman.h collects workarounds for problems in OpenWSMAN = 2.2.6. There are also disabled sections that would use ws_serializer_free_mem but can't because it's broken in OpenWSMAN = 2.2.6. Patches to fix this have been posted upstream. --- po/POTFILES.in|1 + src/Makefile.am | 25 ++- src/hyperv/.gitignore |1 + src/hyperv/hyperv_private.h |7 + src/hyperv/hyperv_wmi.c | 684 + src/hyperv/hyperv_wmi.h | 121 ++ src/hyperv/hyperv_wmi_classes.c | 37 ++ src/hyperv/hyperv_wmi_classes.h | 94 + src/hyperv/hyperv_wmi_generator.input | 294 ++ src/hyperv/hyperv_wmi_generator.py| 309 +++ src/hyperv/openwsman.h| 47 +++ 11 files changed, 1619 insertions(+), 1 deletions(-) create mode 100644 src/hyperv/.gitignore create mode 100644 src/hyperv/hyperv_wmi.c create mode 100644 src/hyperv/hyperv_wmi.h create mode 100644 src/hyperv/hyperv_wmi_classes.c create mode 100644 src/hyperv/hyperv_wmi_classes.h create mode 100644 src/hyperv/hyperv_wmi_generator.input create mode 100755 src/hyperv/hyperv_wmi_generator.py create mode 100644 src/hyperv/openwsman.h diff --git a/po/POTFILES.in b/po/POTFILES.in index b3344b3..848d581 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -29,6 +29,7 @@ src/esx/esx_vi_methods.c src/esx/esx_vi_types.c src/fdstream.c src/hyperv/hyperv_driver.c +src/hyperv/hyperv_wmi.c src/interface/netcf_driver.c src/internal.h src/libvirt.c diff --git a/src/Makefile.am b/src/Makefile.am index 49da107..9971a90 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -387,7 +387,23 @@ HYPERV_DRIVER_SOURCES = \ hyperv/hyperv_storage_driver.c hyperv/hyperv_storage_driver.h \ hyperv/hyperv_device_monitor.c hyperv/hyperv_device_monitor.h \ hyperv/hyperv_secret_driver.c hyperv/hyperv_secret_driver.h \ - hyperv/hyperv_nwfilter_driver.c hyperv/hyperv_nwfilter_driver.h + hyperv/hyperv_nwfilter_driver.c hyperv/hyperv_nwfilter_driver.h \ + hyperv/hyperv_wmi.c hyperv/hyperv_wmi.h \ + hyperv/hyperv_wmi_classes.c hyperv/hyperv_wmi_classes.h \ + hyperv/openwsman.h + +HYPERV_DRIVER_GENERATED = \ + hyperv/hyperv_wmi.generated.c \ + hyperv/hyperv_wmi.generated.h \ + hyperv/hyperv_wmi_classes.generated.c \ + hyperv/hyperv_wmi_classes.generated.h \ + hyperv/hyperv_wmi_classes.generated.typedef + +HYPERV_DRIVER_EXTRA_DIST = \ + hyperv/hyperv_wmi_generator.input \ + hyperv/hyperv_wmi_generator.py \ + $(HYPERV_DRIVER_GENERATED) + NETWORK_DRIVER_SOURCES = \ network/bridge_driver.h network/bridge_driver.c @@ -819,6 +835,12 @@ libvirt_driver_esx_la_DEPENDENCIES = $(ESX_DRIVER_GENERATED) endif +BUILT_SOURCES += $(HYPERV_DRIVER_GENERATED) + +$(HYPERV_DRIVER_GENERATED): $(srcdir)/hyperv/hyperv_wmi_generator.input \ +$(srcdir)/hyperv/hyperv_wmi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/hyperv/hyperv_wmi_generator.py + if WITH_HYPERV if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_hyperv.la @@ -1029,6 +1051,7 @@ EXTRA_DIST += \ $(ESX_DRIVER_SOURCES) \ $(ESX_DRIVER_EXTRA_DIST)\ $(HYPERV_DRIVER_SOURCES)\ + $(HYPERV_DRIVER_EXTRA_DIST) \ $(NETWORK_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(STORAGE_DRIVER_SOURCES) \ diff --git a/src/hyperv/.gitignore b/src/hyperv/.gitignore new file mode 100644 index 000..29e1d48 --- /dev/null +++ b/src/hyperv/.gitignore @@ -0,0 +1 @@ +*.generated.* diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h index 17bdd60..0d5370e 100644 --- a/src/hyperv/hyperv_private.h +++ b/src/hyperv/hyperv_private.h @@ -26,9 +26,16 @@ # include internal.h # include virterror_internal.h +# include openwsman.h # define HYPERV_ERROR(code, ...)
[libvirt] [PATCH 2/5] hyperv: Add driver skeleton
--- cfg.mk |1 + include/libvirt/virterror.h |1 + libvirt.spec.in |9 +++ po/POTFILES.in |1 + src/Makefile.am | 29 + src/driver.h |1 + src/hyperv/hyperv_device_monitor.c | 79 + src/hyperv/hyperv_device_monitor.h | 29 + src/hyperv/hyperv_driver.c | 108 ++ src/hyperv/hyperv_driver.h | 29 + src/hyperv/hyperv_interface_driver.c | 79 + src/hyperv/hyperv_interface_driver.h | 29 + src/hyperv/hyperv_network_driver.c | 79 + src/hyperv/hyperv_network_driver.h | 29 + src/hyperv/hyperv_nwfilter_driver.c | 79 + src/hyperv/hyperv_nwfilter_driver.h | 29 + src/hyperv/hyperv_private.h | 34 +++ src/hyperv/hyperv_secret_driver.c| 79 + src/hyperv/hyperv_secret_driver.h| 29 + src/hyperv/hyperv_storage_driver.c | 79 + src/hyperv/hyperv_storage_driver.h | 29 + src/libvirt.c| 12 src/util/virterror.c |3 + 23 files changed, 876 insertions(+), 0 deletions(-) create mode 100644 src/hyperv/hyperv_device_monitor.c create mode 100644 src/hyperv/hyperv_device_monitor.h create mode 100644 src/hyperv/hyperv_driver.c create mode 100644 src/hyperv/hyperv_driver.h create mode 100644 src/hyperv/hyperv_interface_driver.c create mode 100644 src/hyperv/hyperv_interface_driver.h create mode 100644 src/hyperv/hyperv_network_driver.c create mode 100644 src/hyperv/hyperv_network_driver.h create mode 100644 src/hyperv/hyperv_nwfilter_driver.c create mode 100644 src/hyperv/hyperv_nwfilter_driver.h create mode 100644 src/hyperv/hyperv_private.h create mode 100644 src/hyperv/hyperv_secret_driver.c create mode 100644 src/hyperv/hyperv_secret_driver.h create mode 100644 src/hyperv/hyperv_storage_driver.c create mode 100644 src/hyperv/hyperv_storage_driver.h diff --git a/cfg.mk b/cfg.mk index ffaca85..0c23ab8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -421,6 +421,7 @@ sc_prohibit_xmlGetProp: msg_gen_function = msg_gen_function += ESX_ERROR msg_gen_function += ESX_VI_ERROR +msg_gen_function += HYPERV_ERROR msg_gen_function += PHYP_ERROR msg_gen_function += VIR_ERROR msg_gen_function += VMX_ERROR diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index efa4796..bd64eb4 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -82,6 +82,7 @@ typedef enum { VIR_FROM_EVENT = 40, /* Error from event loop impl */ VIR_FROM_LIBXL = 41, /* Error from libxenlight driver */ VIR_FROM_LOCKING = 42, /* Error from lock manager */ +VIR_FROM_HYPERV = 43, /* Error from Hyper-V driver */ } virErrorDomain; diff --git a/libvirt.spec.in b/libvirt.spec.in index 230237e..c971681 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -50,6 +50,7 @@ # Then the hypervisor drivers that talk a native remote protocol %define with_phyp 0%{!?_without_phyp:1} %define with_esx 0%{!?_without_esx:1} +%define with_hyperv0%{!?_without_hyperv:1} %define with_xenapi0%{!?_without_xenapi:1} # Then the secondary host drivers @@ -437,6 +438,9 @@ BuildRequires: libcurl-devel BuildRequires: curl-devel %endif %endif +%if %{with_hyperv} +BuildRequires: openwsman-devel = 2.2.6 +%endif %if %{with_audit} BuildRequires: audit-libs-devel %endif @@ -569,6 +573,10 @@ of recent versions of Linux (and other OSes). %define _without_esx --without-esx %endif +%if ! %{with_hyperv} +%define _without_hyperv --without-hyperv +%endif + %if ! %{with_vmware} %define _without_vmware --without-vmware %endif @@ -687,6 +695,7 @@ of recent versions of Linux (and other OSes). %{?_without_one} \ %{?_without_phyp} \ %{?_without_esx} \ + %{?_without_hyperv} \ %{?_without_vmware} \ %{?_without_network} \ %{?_with_rhel5_api} \ diff --git a/po/POTFILES.in b/po/POTFILES.in index 5782cbf..b3344b3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -28,6 +28,7 @@ src/esx/esx_vi.c src/esx/esx_vi_methods.c src/esx/esx_vi_types.c src/fdstream.c +src/hyperv/hyperv_driver.c src/interface/netcf_driver.c src/internal.h src/libvirt.c diff --git a/src/Makefile.am b/src/Makefile.am index 39f0cf8..49da107 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -379,6 +379,15 @@ ESX_DRIVER_EXTRA_DIST = \ esx/esx_vi_generator.py \ $(ESX_DRIVER_GENERATED) +HYPERV_DRIVER_SOURCES = \ +
Re: [libvirt] [PATCH 1/5] hyperv: Add configure check for OpenWSMAN
On 07/13/2011 01:01 PM, Matthias Bolte wrote: --- configure.ac | 38 ++ 1 files changed, 38 insertions(+), 0 deletions(-) I'd like to see the libvirt.spec.in changes from patch 2/5 squashed back into this patch - that is, both introduce the new ./configure option, and control whether the new option gets used in an rpm, all in the same patch (even if the option is otherwise a no-op until the rest of patch 2). So I'll review those changes here: diff --git a/libvirt.spec.in b/libvirt.spec.in index 230237e..c971681 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -50,6 +50,7 @@ # Then the hypervisor drivers that talk a native remote protocol %define with_phyp 0%{!?_without_phyp:1} %define with_esx 0%{!?_without_esx:1} +%define with_hyperv0%{!?_without_hyperv:1} %define with_xenapi0%{!?_without_xenapi:1} # Then the secondary host drivers @@ -437,6 +438,9 @@ BuildRequires: libcurl-devel BuildRequires: curl-devel %endif %endif +%if %{with_hyperv} +BuildRequires: openwsman-devel = 2.2.6 +%endif On Fedora, the package is named libwsman-devel (with counterparts openwsman-server, libwsman1, and openwsman-client). So this line needs to be fixed. diff --git a/configure.ac b/configure.ac index e9d5be4..d7ebe79 100644 --- a/configure.ac +++ b/configure.ac @@ -66,6 +66,7 @@ XMLRPC_REQUIRED=1.14.0 HAL_REQUIRED=0.5.0 DEVMAPPER_REQUIRED=1.0.0 LIBCURL_REQUIRED=7.18.0 +OPENWSMAN_REQUIRED=2.2.6 Fedora 14 is only at 2.2.3 (the libwsman-devel package), Fedora 15 at 2.2.4, and rawhide at 2.2.5, which will slightly hamper my ability to test remaining patches (I can inspect them, but can't compile-test them, without installing an out-of-distro build). Are we sure we can't support anything earlier than 2.2.6? Which distros already have 2.2.6 available? But that's not necessarily a show-stopper for this patch. + +if test $with_hyperv = yes || test $with_hyperv = check; then +PKG_CHECK_MODULES(OPENWSMAN, openwsman = $OPENWSMAN_REQUIRED, [ Autoconf suggests using this quoting: PKG_CHECK_MODULES([OPENWSMAN], [openwsman = $OPENWSMAN_REQUIRED], [ ACK with the spec file changes pulled in and nits fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC New virDomainBlockPull API family to libvirt
Unfortunately, after committing the blockPull API to libvirt, the qemu community decided to change the API somewhat and we needed to revert the libvirt implementation. Now that the qemu API is settling out again, I would like to propose an updated libvirt public API which has required only a minor set of changes to sync back up to the qemu API. Summary of changes: - Qemu dropped incremental streaming so remove libvirt incremental BlockPull() API - Rename virDomainBlockPullAll() to virDomainBlockPull() - Changes required to qemu monitor handlers for changed command names Currently, qemu block device streaming completely flattens a disk image's backing file chain. Consider the following chain: A-B-C where C is a leaf image that is backed by B and B is backed by A. The current disk streaming command will produce an independent image C with no backing file. Future versions of qemu block streaming may support an option to specify a new base image from the current chain. For example: stream --backing_file B C would pull all blocks that are only in A to produce the chain: B-C (thus eliminating a dependency on A but maintaining B as a backing image. Do we want to create room in the BlockPull API to support this advanced usage in the future? If so, then a new parameter must be added to BlockPull: const char *backing_path. Even if we extend the API in this manner, the initial implementation will not support it because qemu will not support it immediately, and libvirt is missing core functionality to support it (no public enumeration of the disk backing file chain). Thoughts? -- 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 virDomainBlockPullInfo(). An ongoing stream can be cancelled with virDomainBlockPullAbort(). An event (VIR_DOMAIN_EVENT_ID_BLOCK_PULL) 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 virDomainBlockPullInfo() for completion and could also be used by the security driver to revoke access to the backing file when it is no longer needed. /* * BlockPull API */ /* An iterator for initiating and monitoring block pull operations */ typedef unsigned long long virDomainBlockPullCursor; typedef struct _virDomainBlockPullInfo virDomainBlockPullInfo; struct _virDomainBlockPullInfo { /* * The following fields provide an indication of block pull progress. @cur * indicates the current position and will be between 0 and @end. @end is * the final cursor position for this operation and represents completion. * To approximate progress, divide @cur by @end. */ virDomainBlockPullCursor cur; virDomainBlockPullCursor end; }; typedef virDomainBlockPullInfo *virDomainBlockPullInfoPtr; /** * virDomainBlockPull: * @dom: pointer to domain object * @path: Fully-qualified filename of disk * @flags: currently unused, for future extension * * Populate a disk image with data from its backing image. Once all data from * its backing image has been pulled, the disk no longer depends on a backing * image. This function pulls data for the entire device in the background. * Progress of the operation can be checked with virDomainGetBlockPullInfo() and * the operation can be aborted with virDomainBlockPullAbort(). When finished, * an asynchronous event is raised to indicate the final status. * * Returns 0 if the operation has started, -1 on failure. */ int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned int flags); /** * virDomainBlockPullAbort: * @dom: pointer to domain object * @path: fully-qualified filename of disk * @flags: currently unused, for future extension * * Cancel a pull operation previously started by virDomainBlockPullAll(). * * Returns -1 in case of failure, 0 when successful. */ int virDomainBlockPullAbort(virDomainPtr dom, const char *path, unsigned int flags); /** * virDomainGetBlockPullInfo: * @dom:
Re: [libvirt] [PATCH 2/5] hyperv: Add driver skeleton
On 07/13/2011 01:01 PM, Matthias Bolte wrote: --- cfg.mk |1 + include/libvirt/virterror.h |1 + libvirt.spec.in |9 +++ See patch 1/5 about this file. po/POTFILES.in |1 + src/Makefile.am | 29 + src/driver.h |1 + src/hyperv/hyperv_device_monitor.c | 79 + src/hyperv/hyperv_device_monitor.h | 29 + src/hyperv/hyperv_driver.c | 108 ++ src/hyperv/hyperv_driver.h | 29 + src/hyperv/hyperv_interface_driver.c | 79 + src/hyperv/hyperv_interface_driver.h | 29 + src/hyperv/hyperv_network_driver.c | 79 + src/hyperv/hyperv_network_driver.h | 29 + src/hyperv/hyperv_nwfilter_driver.c | 79 + src/hyperv/hyperv_nwfilter_driver.h | 29 + src/hyperv/hyperv_private.h | 34 +++ src/hyperv/hyperv_secret_driver.c| 79 + src/hyperv/hyperv_secret_driver.h| 29 + src/hyperv/hyperv_storage_driver.c | 79 + src/hyperv/hyperv_storage_driver.h | 29 + src/libvirt.c| 12 src/util/virterror.c |3 + Quite a skeleton, but a lot of it should be fairly simple to review. + +static virDrvOpenStatus +hypervDeviceOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) +{ +virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); Yay - I don't have to rebase my flags patches to fix this. + +static virDeviceMonitor hypervDeviceMonitor = { +Hyper-V, +.open = hypervDeviceOpen, /* 0.9.4 */ +.close = hypervDeviceClose, /* 0.9.4 */ +}; Can't do much, but that's a reasonable first start. :) +++ b/src/hyperv/hyperv_driver.h @@ -0,0 +1,29 @@ + +/* + * hyperv_driver.h: core driver functions for managing Microsoft Hyper-V hosts + * + * Copyright (C) 2011 Matthias Bolte matthias.bo...@googlemail.com + * Copyright (C) 2009 Michael Sievers msiever...@googlemail.com How much of this file is really attributable to Michael, vs. this just being copy and paste? ACK with the copyright nit fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/27] lxc: reject unknown flags
On 07/13/2011 06:21 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: * src/lxc/lxc_driver.c (lxcOpen, lxcDomainSetMemoryParameters) (lxcDomainGetMemoryParameters): Reject unknown flags. * src/lxc/lxc_container.c (lxcContainerStart): Use unsigned flags. @@ -889,7 +889,7 @@ int lxcContainerStart(virDomainDefPtr def, char *ttyPath) { pid_t pid; -int flags; +unsigned int flags; flags is passed to clone that takes int, so rename this to cflags (in line with oflags and fflags) and keep it as int. ACK, with the clone flags as int. Good idea. I squashed this in, then pushed 9 and 10 (8 will be saved for my round 3 posting, since you had more comments on the esx migration flag handling). Slowly making my way through this series... diff --git i/src/lxc/lxc_container.c w/src/lxc/lxc_container.c index ffa2f34..8e1860b 100644 --- i/src/lxc/lxc_container.c +++ w/src/lxc/lxc_container.c @@ -889,7 +889,7 @@ int lxcContainerStart(virDomainDefPtr def, char *ttyPath) { pid_t pid; -unsigned int flags; +int cflags; int stacksize = getpagesize() * 4; char *stack, *stacktop; lxc_child_argv_t args = { def, nveths, veths, control, ttyPath, @@ -902,19 +902,19 @@ int lxcContainerStart(virDomainDefPtr def, } stacktop = stack + stacksize; -flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; +cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; if (userns_supported()) { VIR_DEBUG(Enable user namespaces); -flags |= CLONE_NEWUSER; +cflags |= CLONE_NEWUSER; } if (def-nets != NULL) { VIR_DEBUG(Enable network namespaces); -flags |= CLONE_NEWNET; +cflags |= CLONE_NEWNET; } -pid = clone(lxcContainerChild, stacktop, flags, args); +pid = clone(lxcContainerChild, stacktop, cflags, args); VIR_FREE(stack); VIR_DEBUG(clone() completed, new container PID is %d, pid); -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 14/27] test: reject unknown flags
On 07/13/2011 06:35 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: static int testDomainCoreDump(virDomainPtr domain, const char *to, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { testConnPtr privconn = domain-conn-privateData; int fd = -1; @@ -1912,6 +1914,8 @@ static int testDomainCoreDump(virDomainPtr domain, virDomainEventPtr event = NULL; int ret = -1; +virCheckFlags(0, -1); testDomainCoreDump understands VIR_DUMP_CRASH. Don't get fooled by the ATTRIBUTE_UNUSED :) Good catch. ACK, with testDomainCoreDump fixed. I've pushed 11-14, with this squashed in. diff --git i/src/test/test_driver.c w/src/test/test_driver.c index 5412bff..f3fb320 100644 --- i/src/test/test_driver.c +++ w/src/test/test_driver.c @@ -1919,7 +1919,7 @@ static int testDomainCoreDump(virDomainPtr domain, virDomainEventPtr event = NULL; int ret = -1; -virCheckFlags(0, -1); +virCheckFlags(VIR_DUMP_CRASH, -1); testDriverLock(privconn); privdom = virDomainFindByName(privconn-domains, -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 15/27] uml: reject unknown flags
On 07/13/2011 06:41 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: * src/uml/uml_driver.c (umlOpen, umlDomainGetXMLDesc) (umlDomainBlockPeek): Reject unknown flags. --- src/uml/uml_driver.c | 14 +++--- 1 files changed, 11 insertions(+), 3 deletions(-) @@ -1559,11 +1562,14 @@ cleanup: static char *umlDomainGetXMLDesc(virDomainPtr dom, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) +{ struct uml_driver *driver = dom-conn-privateData; virDomainObjPtr vm; char *ret = NULL; +virCheckFlags(0, NULL); + umlDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); umlDriverUnlock(driver); Don't get fooled by the ATTRIBUTE_UNUSED again. All *DomainGetXMLDesc use virDomainDefFormat and have to accept all flags that virDomainDefFormat accepts. I suggest to recheck your series for this pattern, here it's just the first time that I notice it. Ouch. Good catch, and I'll have to fix that shortly. I'll post the patch before I commit it, but it should be considered a trivial regression fix, so I'll commit it without waiting for review. ACK, with that virCheckFlags loosened correctly. I squashed this in for this patch, and now I'm working on using the same fix for the regression committed in all the prior pushed patches. diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c index 132aef1..da91687 100644 --- i/src/uml/uml_driver.c +++ w/src/uml/uml_driver.c @@ -1599,7 +1599,9 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom, virDomainObjPtr vm; char *ret = NULL; -virCheckFlags(0, NULL); +virCheckFlags(VIR_DOMAIN_XML_SECURE | + VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_UPDATE_CPU, NULL); umlDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/10] network: new XML to support virtual switch functionality
On 07/05/2011 01:45 AM, Laine Stump wrote: The virDomainActualNetDef will only be parsed/formatted if the parse/format function is called with the VIR_DOMAIN_XML_ACTUAL_NET flags set (which is only needed when saving/loading a running domain's state info to the stateDir). To prevent this flag bit from accidentally being used in the public API, a RESERVED placeholder was put into the public flags enum (at the same time, I noticed there was another private flag that hadn't been reserved, so I added that one, making both of these flags #defined from the public RESERVED flags, and since it was also only used in domain_conf.c, I unpolluted domain_conf.h, putting both #defines in domain_conf.c. It turns out that we've used internal-use flags before. See how libvirt.c filters out flags in both virDomainGetXMLDesc and virSecretGetValue if the flags are larger than 0x, so that it can start internal flags at 116. Regarding the networking code and our discussions on whether we should split out a second internalFlags argument rather than cramming internal and external flags into a single parameter, I think we should be consistent. That is, either the existing uses of VIR_SECRET_GET_VALUE_INTERNAL_CALL and VIR_DOMAIN_XML_INTERNAL_STATUS should be factored into internalFlags arguments, or your new patches for virtual switches should define VIR_DOMAIN_XML_ACTUAL_NET to be placed alongside VIR_DOMAIN_XML_INTERNAL_STATUS +++ b/include/libvirt/libvirt.h.in @@ -1112,6 +1112,8 @@ typedef enum { VIR_DOMAIN_XML_SECURE = (1 0), /* dump security sensitive information too */ VIR_DOMAIN_XML_INACTIVE = (1 1), /* dump inactive domain information */ VIR_DOMAIN_XML_UPDATE_CPU = (1 2), /* update guest CPU requirements according to host CPU */ +VIR_DOMAIN_XML_RESERVED1= (1 30), /* reserved for internal used */ +VIR_DOMAIN_XML_RESERVED2= (1 31), /* reserved for internal used */ } virDomainXMLFlags; -/* Private component of virDomainXMLFlags */ -typedef enum { - VIR_DOMAIN_XML_INTERNAL_STATUS = (116), /* dump internal domain status information */ -} virDomainXMLInternalFlags; - Meanwhile, I've got a patch to libvirt.c; I think virDomainGetXMLDesc should reject an attempt to pass 116, rather than silently ignore it. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 15/27] uml: reject unknown flags
On 07/13/2011 03:14 PM, Eric Blake wrote: Don't get fooled by the ATTRIBUTE_UNUSED again. All *DomainGetXMLDesc use virDomainDefFormat and have to accept all flags that virDomainDefFormat accepts. I suggest to recheck your series for this pattern, here it's just the first time that I notice it. Ouch. Good catch, and I'll have to fix that shortly. I'll post the patch before I commit it, but it should be considered a trivial regression fix, so I'll commit it without waiting for review. Thanks for making me audit all the drivers one more time. It turns out that none of the other drivers committed thus far had this problem (rather, their problem was that virDomainGetXMLDesc didn't have any virCheckFlags in the first place) - for example, libxlDomainGetXMLDesc. So without a regression, I can't claim the trivial rule any more, and will just add the new patch to my v3 series. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 24/27] build: don't hand-roll cloexec code
On 07/13/2011 07:22 AM, Matthias Bolte wrote: 2011/7/8 Eric Blake ebl...@redhat.com: No need to repeat common code. * src/util/bridge.c (brInit): Use virSetCloseExec. (brSetInterfaceUp): Prefer unsigned flags. * src/uml/uml_driver.c (umlSetCloseExec): Delete. (umlStartVMDaemon): Use util version instead. --- v2: new patch I've pushed 20-23. - -if (VIR_ALLOC(*ctlp) 0) { +if (fd 0 || +virSetCloseExec(fd) 0 || +VIR_ALLOC(*ctlp) 0) { VIR_FORCE_CLOSE(fd); -return ENOMEM; +return errno; } Is it guaranteed that calloc will set ENOMEM, or do we need some gnulib module to guarantee this? Well what do you know. We need to import 'calloc-posix' to get that to happen on mingw. :) (*ctlp)-fd = fd; @@ -599,7 +590,7 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; -int flags; +unsigned int flags; flags is used used with ifr.ifr_flags that is signed (actually it's a short). So I'd prefer renaming it to ifflags and keep it as int. ACK, with that questions/comments answered/addressed. Here's what I squashed in before pushing. diff --git i/bootstrap.conf w/bootstrap.conf index 3c3d0e0..2fc457e 100644 --- i/bootstrap.conf +++ w/bootstrap.conf @@ -27,6 +27,7 @@ byteswap c-ctype c-strcase c-strcasestr +calloc-posix canonicalize-lgpl chown close diff --git i/src/util/bridge.c w/src/util/bridge.c index a6b5768..0f4b639 100644 --- i/src/util/bridge.c +++ w/src/util/bridge.c @@ -590,7 +590,7 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; -unsigned int flags; +int ifflags; if (!ctl || !ifname) return EINVAL; @@ -603,10 +603,10 @@ brSetInterfaceUp(brControl *ctl, if (ioctl(ctl-fd, SIOCGIFFLAGS, ifr) 0) return errno; -flags = up ? (ifr.ifr_flags | IFF_UP) : (ifr.ifr_flags ~IFF_UP); +ifflags = up ? (ifr.ifr_flags | IFF_UP) : (ifr.ifr_flags ~IFF_UP); -if (ifr.ifr_flags != flags) { -ifr.ifr_flags = flags; +if (ifr.ifr_flags != ifflags) { +ifr.ifr_flags = ifflags; if (ioctl(ctl-fd, SIOCSIFFLAGS, ifr) 0) return errno; -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix a regression of attaching device
At 07/13/2011 11:03 PM, Eric Blake Write: On 07/13/2011 09:17 AM, Osier Yang wrote: qemuDomainModifyDeviceFlags: Changing ret to 0, but don't reset it to -1 on failure, it's not good idea to change the value of ret in the codes to do some condition checking. This patch fix it. Can you identify which commit introduced the regression, and what behavior actually broke as a result? The commit message will be more useful with that information. Commit da1eba6b introduced this regression. I can reproduce this bug as the following: # cat usb.xml #The format of the xml file is wrong. disk type='file' device='disk' driver name='qemu' type='raw'/ source file='/var/lib/libvirt/images/test2.img'/ target dev='ubdb' bus='usb'/ disk # virsh attach-device vm1 usb.xml Device attached successfully The command successed, but it failed actually. --- src/qemu/qemu_driver.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a6b48e..ae11c68 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4892,10 +4892,10 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, _(unknown domain modify action %d), action); break; } -} else -ret = 0; +} -if (!ret (flags VIR_DOMAIN_AFFECT_LIVE)) { +if (!(flags VIR_DOMAIN_AFFECT_CONFIG) +(flags VIR_DOMAIN_AFFECT_LIVE)) { I'm not sure I agree with this change. The logic flow before this patch is: I do not agree with this change too. if config make the change in a copy, or record that the change failed if live, and either no config or change to config copy is happy make the change live, or record that change failed if config, and either no live or change to live is happy commit the changed copy made earlier Your proposed change makes it so that we can now change live even if we are going to fail to change the config, which is not a good idea. Why not instead do: if config { make change in a copy if ret 0 goto error } if live { make change in live if ret 0 goto error } if config commit the changed copy made earlier Agree with this way, but I have anohter simple way to fix this problem: if !ret live { reset ret to -1 ... } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix a regression of attaching device
于 2011年07月13日 23:03, Eric Blake 写道: On 07/13/2011 09:17 AM, Osier Yang wrote: qemuDomainModifyDeviceFlags: Changing ret to 0, but don't reset it to -1 on failure, it's not good idea to change the value of ret in the codes to do some condition checking. This patch fix it. Can you identify which commit introduced the regression, and what behavior actually broke as a result? The commit message will be more useful with that information. --- src/qemu/qemu_driver.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a6b48e..ae11c68 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4892,10 +4892,10 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, _(unknown domain modify action %d), action); break; } -} else -ret = 0; +} -if (!ret (flags VIR_DOMAIN_AFFECT_LIVE)) { +if (!(flags VIR_DOMAIN_AFFECT_CONFIG) +(flags VIR_DOMAIN_AFFECT_LIVE)) { I'm not sure I agree with this change. The logic flow before this patch is: if config make the change in a copy, or record that the change failed if live, and either no config or change to config copy is happy make the change live, or record that change failed if config, and either no live or change to live is happy commit the changed copy made earlier Your proposed change makes it so that we can now change live even if we are going to fail to change the config, which is not a good idea. Yes, I didn't follow with the complete meaning of commit da1eba6b pointed by Wen, a updated patch is following. Why not instead do: if config { make change in a copy if ret 0 goto error } if live { make change in live if ret 0 goto error } if config commit the changed copy made earlier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4 V4] Add support for send keys to guest
Add virtkey lib for usage-improvment and keycode translating. Expose send-key in virsh Implement send-key function for the qemu driver Daniel P. Berrange (1): util: Add keymaps.csv Lai Jiangshan (3): util: add virtkey send-key: Expose the new API in virsh qemu:send-key: Implement the driver methods include/libvirt/libvirt.h.in |8 + src/Makefile.am | 11 +- src/libvirt_private.syms |5 + src/qemu/qemu_driver.c | 71 +++ src/qemu/qemu_monitor.c | 17 ++ src/qemu/qemu_monitor.h |5 + src/qemu/qemu_monitor_json.c | 15 ++ src/qemu/qemu_monitor_json.h |5 + src/qemu/qemu_monitor_text.c | 49 + src/qemu/qemu_monitor_text.h |5 + src/util/keymaps.csv | 463 ++ src/util/virtkey.c | 117 +++ src/util/virtkey.h | 41 src/util/virtkeymap-gen.py | 47 + tools/virsh.c| 94 + tools/virsh.pod |4 + 16 files changed, 956 insertions(+), 1 deletions(-) create mode 100644 src/util/keymaps.csv create mode 100644 src/util/virtkey.c create mode 100644 src/util/virtkey.h create mode 100644 src/util/virtkeymap-gen.py -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4 V4] util: add virtkey
Add virtkey lib for usage-improvment and keycode translating. Add 4 internal API for the aim const char *virKeycodeSetTypeToString(int codeset); int virKeycodeSetTypeFromString(const char *name); int virParseKeyName(virKeycodeSet codeset, const char *keyname); int virTranslateKeyCode(virKeycodeSet from_codeset, virKeycodeSet to_offset, int key_value); Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- include/libvirt/libvirt.h.in |8 +++ src/Makefile.am | 11 - src/libvirt_private.syms |5 ++ src/util/virtkey.c | 117 ++ src/util/virtkey.h | 41 +++ src/util/virtkeymap-gen.py | 47 + 6 files changed, 228 insertions(+), 1 deletions(-) create mode 100644 src/util/virtkey.c create mode 100644 src/util/virtkey.h create mode 100644 src/util/virtkeymap-gen.py diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index d5a7105..acfe9d9 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1778,6 +1778,14 @@ typedef enum { VIR_KEYCODE_SET_ATSET1 = 2, VIR_KEYCODE_SET_ATSET2 = 3, VIR_KEYCODE_SET_ATSET3 = 4, +VIR_KEYCODE_SET_OSX= 5, +VIR_KEYCODE_SET_XT_KBD = 6, +VIR_KEYCODE_SET_USB= 7, +VIR_KEYCODE_SET_WIN32 = 8, +VIR_KEYCODE_SET_XWIN_XT= 9, +VIR_KEYCODE_SET_XFREE86_KBD_XT = 10, + +VIR_KEYCODE_SET_LAST, } virKeycodeSet; /** diff --git a/src/Makefile.am b/src/Makefile.am index 39f0cf8..90b0743 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -81,7 +81,16 @@ UTIL_SOURCES = \ util/util.c util/util.h \ util/viraudit.c util/viraudit.h \ util/xml.c util/xml.h \ - util/virterror.c util/virterror_internal.h + util/virterror.c util/virterror_internal.h \ + util/virtkey.c util/virtkey.h \ + util/virtkeymaps.c + +EXTRA_DIST += $(srcdir)/util/virtkeymaps.c $(srcdir)/util/keymaps.csv \ + $(srcdir)/util/virtkeymap-gen.py + +$(srcdir)/util/virtkeymaps.c: $(srcdir)/util/keymaps.csv \ + $(srcdir)/util/virtkeymap-gen.py + python $(srcdir)/util/virtkeymap-gen.py $(srcdir)/util/keymaps.csv $@ EXTRA_DIST += util/threads-pthread.c util/threads-win32.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3237d18..6611471 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1094,6 +1094,11 @@ virSetError; virSetErrorLogPriorityFunc; virStrerror; +# virtkey.h +virKeycodeSetTypeToString; +virKeycodeSetTypeFromString; +virParseKeyName; +virTranslateKeyCode; # xml.h virXMLParseFileHelper; diff --git a/src/util/virtkey.c b/src/util/virtkey.c new file mode 100644 index 000..1eb3c62 --- /dev/null +++ b/src/util/virtkey.c @@ -0,0 +1,117 @@ + +/* + * Copyright (c) 2011 Lai Jiangshan + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + */ + +#include virtkey.h +#include string.h +#include stddef.h + +#define getfield(object, field_type, field_offset) \ +(*(typeof(field_type) *)((char *)(object) + field_offset)) + +static unsigned int codeOffset[] = { +[VIR_KEYCODE_SET_LINUX] = +offsetof(struct keycode, linux_keycode), +[VIR_KEYCODE_SET_XT] = +offsetof(struct keycode, xt), +[VIR_KEYCODE_SET_ATSET1] = +offsetof(struct keycode, atset1), +[VIR_KEYCODE_SET_ATSET2] = +offsetof(struct keycode, atset2), +[VIR_KEYCODE_SET_ATSET3] = +offsetof(struct keycode, atset3), +[VIR_KEYCODE_SET_OSX] = +offsetof(struct keycode, os_x), +[VIR_KEYCODE_SET_XT_KBD] = +offsetof(struct keycode, xt_kbd), +[VIR_KEYCODE_SET_USB] = +offsetof(struct keycode, usb), +[VIR_KEYCODE_SET_WIN32] = +offsetof(struct keycode, win32), +[VIR_KEYCODE_SET_XWIN_XT] = +offsetof(struct keycode, xwin_xt), +[VIR_KEYCODE_SET_XFREE86_KBD_XT] = +offsetof(struct keycode, xfree86_kbd_xt), +}; + +VIR_ENUM_IMPL(virKeycodeSet, VIR_KEYCODE_SET_LAST, +linux, +xt, +atset1, +atset2, +atset3, +os_x, +xt_kbd, +usb, +win32, +xwin_xt, +xfree86_kbd_xt); + +static int virParseKeyNameOffset(unsigned int name_offset, + unsigned int code_offset, + const char *keyname) +{ +int i; + +for (i = 0; i virtKeycodesSize; i++) { +const char *name = getfield(virtKeycodes + i, const char *, name_offset); + +if (name !strcmp(name, keyname)) +
[libvirt] [PATCH 1/4 V4] util: Add keymaps.csv
From: Daniel P. Berrange berra...@redhat.com Should keep it as the same as: http://git.gnome.org/browse/gtk-vnc/commit/src/keymaps.csv All master keymaps are defined in a CSV file. THis covers Linux keycodes, OSX keycodes, AT set1, 2 3, XT keycodes, the XT encoding used by the Linux KBD driver, USB keycodes, Win32 keycodes, the XT encoding used by Xorg on Cygwin, the XT encoding used by Xorg on Linux with kbd driver. Signed-off-by: Daniel P. Berrange berra...@redhat.com Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- src/util/keymaps.csv | 463 ++ 1 files changed, 463 insertions(+), 0 deletions(-) create mode 100644 src/util/keymaps.csv diff --git a/src/util/keymaps.csv b/src/util/keymaps.csv new file mode 100644 index 000..c447f46 --- /dev/null +++ b/src/util/keymaps.csv @@ -0,0 +1,463 @@ +Linux Name,Linux Keycode,OS-X Name,OS-X Keycode,AT set1 keycode,AT set2 keycode,AT set3 keycode,XT,XT KBD,USB Keycodes,Win32 Name,Win32 Keycode,Xwin XT,Xfree86 KBD XT +KEY_RESERVED,0 +KEY_ESC,1,Escape,0x35,1,118,8,1,1,41,VK_ESCAPE,0x1b,1,1 +KEY_1,2,ANSI_1,0x12,2,22,22,2,2,30,VK_1,0x31,2,2 +KEY_2,3,ANSI_2,0x13,3,30,30,3,3,31,VK_2,0x32,3,3 +KEY_3,4,ANSI_3,0x14,4,38,38,4,4,32,VK_3,0x33,4,4 +KEY_4,5,ANSI_4,0x15,5,37,37,5,5,33,VK_4,0x34,5,5 +KEY_5,6,ANSI_5,0x17,6,46,46,6,6,34,VK_5,0x35,6,6 +KEY_6,7,ANSI_6,0x16,7,54,54,7,7,35,VK_6,0x36,7,7 +KEY_7,8,ANSI_7,0x1a,8,61,61,8,8,36,VK_7,0x37,8,8 +KEY_8,9,ANSI_8,0x1c,9,62,62,9,9,37,VK_8,0x38,9,9 +KEY_9,10,ANSI_9,0x19,10,70,70,10,10,38,VK_9,0x39,10,10 +KEY_0,11,ANSI_0,0x1d,11,69,69,11,11,39,VK_0,0x30,11,11 +KEY_MINUS,12,ANSI_Minus,0x1b,12,78,78,12,12,45,VK_OEM_MINUS,0xbd,12,12 +KEY_EQUAL,13,ANSI_Equal,0x18,13,85,85,13,13,46,VK_OEM_PLUS,0xbb,13,13 +KEY_BACKSPACE,14,Delete,0x33,14,102,102,14,14,42,VK_BACK,0x08,14,14 +KEY_TAB,15,Tab,0x30,15,13,13,15,15,43,VK_TAB,0x09,15,15 +KEY_Q,16,ANSI_Q,0xc,16,21,21,16,16,20,VK_Q,0x51,16,16 +KEY_W,17,ANSI_W,0xd,17,29,29,17,17,26,VK_W,0x57,17,17 +KEY_E,18,ANSI_E,0xe,18,36,36,18,18,8,VK_E,0x45,18,18 +KEY_R,19,ANSI_R,0xf,19,45,45,19,19,21,VK_R,0x52,19,19 +KEY_T,20,ANSI_T,0x11,20,44,44,20,20,23,VK_T,0x54,20,20 +KEY_Y,21,ANSI_Y,0x10,21,53,53,21,21,28,VK_Y,0x59,21,21 +KEY_U,22,ANSI_U,0x20,22,60,60,22,22,24,VK_U,0x55,22,22 +KEY_I,23,ANSI_I,0x22,23,67,67,23,23,12,VK_I,0x49,23,23 +KEY_O,24,ANSI_O,0x1f,24,68,68,24,24,18,VK_O,0x4f,24,24 +KEY_P,25,ANSI_P,0x23,25,77,77,25,25,19,VK_P,0x50,25,25 +KEY_LEFTBRACE,26,ANSI_LeftBracket,0x21,26,84,84,26,26,47,VK_OEM_4,0xdb,26,26 +KEY_RIGHTBRACE,27,ANSI_RightBracket,0x1e,27,91,91,27,27,48,VK_OEM_6,0xdd,27,27 +KEY_ENTER,28,Return,0x24,28,90,90,28,28,40,VK_RETURN,0x0d,28,28 +KEY_LEFTCTRL,29,Control,0x3b,29,20,17,29,29,224,VK_LCONTROL,0xa2,29,29 +KEY_LEFTCTRL,29,Control,0x3b,29,20,17,29,29,224,VK_CONTROL,0x11,29,29 +KEY_A,30,ANSI_A,0x0,30,28,28,30,30,4,VK_A,0x41,30,30 +KEY_S,31,ANSI_S,0x1,31,27,27,31,31,22,VK_S,0x53,31,31 +KEY_D,32,ANSI_D,0x2,32,35,35,32,32,7,VK_D,0x44,32,32 +KEY_F,33,ANSI_F,0x3,33,43,43,33,33,9,VK_F,0x46,33,33 +KEY_G,34,ANSI_G,0x5,34,52,52,34,34,10,VK_G,0x47,34,34 +KEY_H,35,ANSI_H,0x4,35,51,51,35,35,11,VK_H,0x48,35,35 +KEY_J,36,ANSI_J,0x26,36,59,59,36,36,13,VK_J,0x4a,36,36 +KEY_K,37,ANSI_K,0x28,37,66,66,37,37,14,VK_K,0x4b,37,37 +KEY_L,38,ANSI_L,0x25,38,75,75,38,38,15,VK_L,0x4c,38,38 +KEY_SEMICOLON,39,ANSI_Semicolon,0x29,39,76,76,39,39,51,VK_OEM_1,0xba,39,39 +KEY_APOSTROPHE,40,ANSI_Quote,0x27,40,82,82,40,40,52,VK_OEM_2,0xbf,40,40 +KEY_GRAVE,41,ANSI_Grave,0x32,41,14,14,41,41,53,VK_OEM_3,0xc0,41,41 +KEY_LEFTSHIFT,42,Shift,0x38,42,18,18,42,42,225,VK_LSHIFT,0xa0,42,42 +KEY_BACKSLASH,43,ANSI_Backslash,0x2a,43,93,93,43,43,50,VK_OEM_5,0xdc,43,43 +KEY_Z,44,ANSI_Z,0x6,44,26,26,44,44,29,VK_Z,0x5a,44,44 +KEY_X,45,ANSI_X,0x7,45,34,34,45,45,27,VK_X,0x58,45,45 +KEY_C,46,ANSI_C,0x8,46,33,33,46,46,6,VK_C,0x43,46,46 +KEY_V,47,ANSI_V,0x9,47,42,42,47,47,25,VK_V,0x56,47,47 +KEY_B,48,ANSI_B,0xb,48,50,50,48,48,5,VK_B,0x42,48,48 +KEY_N,49,ANSI_N,0x2d,49,49,49,49,49,17,VK_N,0x4e,49,49 +KEY_M,50,ANSI_M,0x2e,50,58,58,50,50,16,VK_M,0x4d,50,50 +KEY_COMMA,51,ANSI_Comma,0x2b,51,65,65,51,51,54,VK_OEM_COMMA,0xbc,51,51 +KEY_DOT,52,ANSI_Period,0x2f,52,73,73,52,52,55,VK_OEM_PERIOD,0xbe,52,52 +KEY_SLASH,53,ANSI_Slash,0x2c,53,74,74,53,53,56,VK_OEM_2,0xbf,53,53 +KEY_RIGHTSHIFT,54,RightShift,0x3c,54,89,89,54,54,229,VK_RSHIFT,0xa1,54,54 +KEY_KPASTERISK,55,ANSI_KeypadMultiply,0x43,55,124,126,55,55,85,VK_MULTIPLY,0x6a,55,55 +KEY_LEFTALT,56,Option,0x3a,56,17,25,56,56,226,VK_LMENU,0xa4,56,56 +KEY_LEFTALT,56,Option,0x3a,56,17,25,56,56,226,VK_MENU,0x12,56,56 +KEY_SPACE,57,Space,0x31,57,41,41,57,57,44,VK_SPACE,0x20,57,57 +KEY_CAPSLOCK,58,CapsLock,0x39,58,88,20,58,58,57,VK_CAPITAL,0x14,58,58 +KEY_F1,59,F1,0x7a,59,5,7,59,59,58,VK_F1,0x70,59,59 +KEY_F2,60,F2,0x78,60,6,15,60,60,59,VK_F2,0x71,60,60 +KEY_F3,61,F3,0x63,61,4,23,61,61,60,VK_F3,0x72,61,61 +KEY_F4,62,F4,0x76,62,12,31,62,62,61,VK_F4,0x73,62,62 +KEY_F5,63,F5,0x60,63,3,39,63,63,62,VK_F5,0x74,63,63
[libvirt] [PATCH 4/4 V4] qemu:send-key: Implement the driver methods
qemu driver just accept xt_kbd codeset's keycode, so the lib virtkey is used for translating keycodes from other codesets. Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com --- src/qemu/qemu_driver.c | 71 ++ src/qemu/qemu_monitor.c | 17 ++ src/qemu/qemu_monitor.h |5 +++ src/qemu/qemu_monitor_json.c | 15 + src/qemu/qemu_monitor_json.h |5 +++ src/qemu/qemu_monitor_text.c | 49 + src/qemu/qemu_monitor_text.h |5 +++ 7 files changed, 167 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0ea182d..c93885b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -87,6 +87,7 @@ #include configmake.h #include threadpool.h #include locking/lock_manager.h +#include virtkey.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1843,6 +1844,75 @@ cleanup: return ret; } +static int qemuDomainSendKey(virDomainPtr domain, + unsigned int codeset, + unsigned int holdtime, + unsigned int *keycodes, + int nkeycodes, + unsigned int flags) +{ +struct qemud_driver *driver = domain-conn-privateData; +virDomainObjPtr vm = NULL; +int ret = -1; +qemuDomainObjPrivatePtr priv; + +virCheckFlags(0, -1); + +/* translate the keycode to XT_KBD for qemu driver */ +if (codeset != VIR_KEYCODE_SET_XT_KBD) { +int i; +int keycode; + +for (i = 0; i nkeycodes; i++) { +keycode = virTranslateKeyCode(codeset, VIR_KEYCODE_SET_XT_KBD, + keycodes[i]); +if (keycode 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, can not translate +keycode %u of %s codeset to xt_kbd codeset +keycode, keycodes[i], +virKeycodeSetTypeToString(codeset)); +return -1; +} +keycodes[i] = keycode; +} +} + +qemuDriverLock(driver); +vm = virDomainFindByUUID(driver-domains, domain-uuid); +if (!vm) { +char uuidstr[VIR_UUID_STRING_BUFLEN]; +virUUIDFormat(domain-uuid, uuidstr); +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +priv = vm-privateData; + +if (qemuDomainObjBeginJobWithDriver(driver, vm) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +%s, _(domain is not running)); +goto cleanup; +} + +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorSendKey(priv-mon, holdtime, keycodes, nkeycodes); +qemuDomainObjExitMonitorWithDriver(driver, vm); +if (qemuDomainObjEndJob(vm) == 0) { +vm = NULL; +goto cleanup; +} + +cleanup: +if (vm) +virDomainObjUnlock(vm); +qemuDriverUnlock(driver); +return ret; +} + static int qemudDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { @@ -8646,6 +8716,7 @@ static virDriver qemuDriver = { .domainMigratePerform3 = qemuDomainMigratePerform3, /* 0.9.2 */ .domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */ +.domainSendKey = qemuDomainSendKey, /* 0.9.3 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e593642..35fb999 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2398,6 +2398,23 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon) return ret; } +int qemuMonitorSendKey(qemuMonitorPtr mon, + unsigned int holdtime, + unsigned int *keycodes, + unsigned int nkeycodes) +{ +int ret; + +VIR_DEBUG(mon=%p, holdtime=%u, nkeycodes=%u, + mon, holdtime, nkeycodes); + +if (mon-json) +ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); +else +ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes); +return ret; +} + int qemuMonitorScreendump(qemuMonitorPtr mon, const char *file) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 893f3e9..73a95f1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -444,6 +444,11 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon); int qemuMonitorScreendump(qemuMonitorPtr mon, const char *file); +int qemuMonitorSendKey(qemuMonitorPtr mon, + unsigned int holdtime, + unsigned int *keycodes, + unsigned int nkeycodes); + /** * When running two dd process and