Re: [libvirt] [PATCH v2] add --cache, --serial, --sharable and --pciaddress options(was Re: [PATCH 1/3] add --cache option for attach-disk)

2011-07-13 Thread Hu Tao
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

2011-07-13 Thread Wen Congyang
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}

2011-07-13 Thread Jiri Denemark
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

2011-07-13 Thread Jiri Denemark
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

2011-07-13 Thread Stefan Hajnoczi
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

2011-07-13 Thread Jiri Denemark
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

2011-07-13 Thread 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)
+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

2011-07-13 Thread Nikunj A. Dadhania
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

2011-07-13 Thread Wen Congyang
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

2011-07-13 Thread Andrew Beekhof
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

2011-07-13 Thread Andrew Beekhof
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

2011-07-13 Thread Andrew Beekhof
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-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Jiri Denemark
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

2011-07-13 Thread Zane Bitter

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

2011-07-13 Thread Osier Yang
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

2011-07-13 Thread Osier Yang
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-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Osier Yang
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

2011-07-13 Thread Osier Yang
---
 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

2011-07-13 Thread Osier Yang
---
 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

2011-07-13 Thread Osier Yang
---
 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

2011-07-13 Thread Osier Yang
---
 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

2011-07-13 Thread Osier Yang
---
 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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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 Thread Osier Yang

于 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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Dave Allan
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

2011-07-13 Thread Eric Blake
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-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Eric Blake
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-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Jiri Denemark
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Jiri Denemark
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

2011-07-13 Thread Zane Bitter
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Osier Yang
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread 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.

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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Michal Privoznik
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Matthias Bolte
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)

2011-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Matthias Bolte
---
 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

2011-07-13 Thread Matthias Bolte
---
 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-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Matthias Bolte
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

2011-07-13 Thread Matthias Bolte
---
 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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Adam Litke
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Eric Blake
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

2011-07-13 Thread Wen Congyang
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 Thread Osier Yang

于 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

2011-07-13 Thread Lai Jiangshan
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

2011-07-13 Thread Lai Jiangshan
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

2011-07-13 Thread Lai Jiangshan
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

2011-07-13 Thread Lai Jiangshan
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 

  1   2   >