Re: [libvirt] [PATCH] qemu: Fix (managed)save and snapshots with host mode CPU

2012-03-13 Thread Jiri Denemark
On Mon, Mar 12, 2012 at 17:32:45 -0600, Eric Blake wrote:
 On 03/12/2012 08:31 AM, Jiri Denemark wrote:
  When host-model and host-passthrouh CPU modes were introduced, qemu
  driver was properly modify to update guest CPU definition during
  migration so that we use the right CPU at the destination. However,
  similar treatment is needed for (managed)save and snapshots since they
  need to save the exact CPU so that a domain can be properly restored.
  To avoid repetition of such situation, all places that need live XML
  share the code which generates it.
  
  As a side effect, this patch fixes error reporting from
  qemuDomainSnapshotWriteMetadata().
  ---
   src/conf/domain_conf.c|3 ++-
   src/qemu/qemu_domain.c|   23 +++
   src/qemu/qemu_domain.h|4 
   src/qemu/qemu_driver.c|9 +++--
   src/qemu/qemu_migration.c |8 ++--
   5 files changed, 30 insertions(+), 17 deletions(-)
 
 I haven't tested this as much as I'd like, but by inspection, it looks
 right, and I'd rather get it in now so we maximize our testing time.

I tested managedsave and basic snapshot of a running domain and all was
working fine. However, snapshots are so complicated I didn't even try to test
all possibilities there.

 ACK.

Thanks and pushed.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Qemu-devel] Modern CPU models cannot be used with libvirt

2012-03-13 Thread Itamar Heim

On 03/12/2012 10:19 PM, Ayal Baron wrote:



- Original Message -

On 03/12/2012 02:12 PM, Itamar Heim wrote:

On 03/12/2012 09:01 PM, Anthony Liguori wrote:


It's a trade off. From a RAS perspective, it's helpful to have
information about the host available in the guest.

If you're already exposing a compatible family, exposing the
actual
processor seems to be worth the extra effort.


only if the entire cluster is (and will be?) identical cpu.


At least in my experience, this isn't unusual.


I can definitely see places choosing homogeneous hardware and upgrading every 
few years.
Giving them max capabilities for their cluster sounds logical to me.
Esp. cloud providers.


they would get same performance as from the matching cpu family.
only difference would be if the guest known the name of the host cpu.






or if you don't care about live migration i guess, which could be
hte case for
clouds, then again, not sure a cloud provider would want to expose
the physical
cpu to the tenant.


Depends on the type of cloud you're building, I guess.



Wouldn't this affect a simple startup of a VM with a different CPU (if 
motherboard changed as well cause reactivation issues in windows and fun things 
like that)?


that's an interesting question, I have to assume this works though, 
since we didn't see issues with changing the cpu family for guests so far.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] libvirt hooks stopped vs crashed

2012-03-13 Thread Ante Karamatic
Hi all

I've noticed libvirt's qemu hook doesn't make a difference between
crashed and stopped VM. While I do understand that crashed VM is
essentially a stopped VM, I'd be interested in providing (and working
on) a patch that would differentiate these two cases.

What I'm interested in is if there's a reason, unknown to me, why this
wasn't implemented in the first place. And of course, does it make sense
to differentiate those two.

Thank you!

-- 
Ante Karamatic

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] libvirt hooks stopped vs crashed

2012-03-13 Thread Osier Yang

On 03/13/2012 05:00 PM, Ante Karamatic wrote:

Hi all

I've noticed libvirt's qemu hook doesn't make a difference between
crashed and stopped VM. While I do understand that crashed VM is
essentially a stopped VM, I'd be interested in providing (and working
on) a patch that would differentiate these two cases.

What I'm interested in is if there's a reason, unknown to me, why this
wasn't implemented in the first place. And of course, does it make sense
to differentiate those two.

Thank you!



There are reasons defined for a stopped VM:

typedef enum {
VIR_DOMAIN_SHUTOFF_UNKNOWN = 0, /* the reason is unknown */
VIR_DOMAIN_SHUTOFF_SHUTDOWN = 1,/* normal shutdown */
VIR_DOMAIN_SHUTOFF_DESTROYED = 2,   /* forced poweroff */
VIR_DOMAIN_SHUTOFF_CRASHED = 3, /* domain crashed */
VIR_DOMAIN_SHUTOFF_MIGRATED = 4,/* migrated to another host */
VIR_DOMAIN_SHUTOFF_SAVED = 5,   /* saved to a file */
VIR_DOMAIN_SHUTOFF_FAILED = 6,  /* domain failed to start */
VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT = 7, /* restored from a snapshot 
which was
   * taken while domain was 
shutoff */

#ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_SHUTOFF_LAST
#endif
} virDomainShutoffReason;

A rough thought is you can take use of the parameter @extra
to pass the a string to represent the shutoff reason.

snip
 * @op: the operation on the id e.g. VIR_HOOK_QEMU_OP_START
 * @sub_op: a sub_operation, currently unused
 * @extra: optional string information
snip

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/9] qemu: Do not start with source for removable disks if tray is open

2012-03-13 Thread Osier Yang

On 03/12/2012 05:10 PM, Paolo Bonzini wrote:

Il 12/03/2012 10:28, Osier Yang ha scritto:

VMs with physical CD-ROMs in general should not be migrated, so I think
migration is not a problem in this case.


QEMU will prohibit that, right? if so, we have no problem here.
Either migrate or (save + restore).


Again, it will not but it should, so for migration it's ok.  When you
start the domain, we could just say tray=open is invalid fordisk
type=block.



We shouldn't do that while conf parsing, the only place which
might be proper is while building the qemu command line, but
I'm not quite sure if it's good to do that in libvirt, it's
more like a protection for the qemu problem. Anyway, I created
the patch, which will be posted along with the v2 series together
later, let's evaluate there then. :-)

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] These two machines look like they have dontaudit rules disabled.

2012-03-13 Thread Daniel J Walsh
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

audit_log-ex-std-node22.prod.rhcloud.com-2012-03-12
audit_log-ex-std-node24.prod.rhcloud.com-2012-03-12


semodule -B

Will turn dontaudit rules back on.

22:31:32.791:507663) : avc:  denied  { siginh } for  pid=15258
comm=trap-user scontext=system_u:system_r:sshd_t:s0-s0:c0.c1023
tcontext=unconfined_u:system_r:libra_t:s0:c5,c641 tclass=process

grep siginh * | audit2allow


#= sshd_t ==
# This avc has a dontaudit rule in the current policy

allow sshd_t libra_t:process siginh;
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk9fIj4ACgkQrlYvE4MpobM44gCeJEqC+EV3HN57pL2j/hv9hMYO
cewAnjYiI6hehUpwqVEQJ3bX4Dz3eS95
=GqCQ
-END PGP SIGNATURE-

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Postgresql binding to other localhosts by libra instances.

2012-03-13 Thread Daniel J Walsh
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

I see several postgresql instances trying to bind to 127.0.0.1

audit_log-ex-lg-node4.prod.rhcloud.com-2012-03-12
audit_log-ex-std-node18.prod.rhcloud.com-2012-03-12
audit_log-ex-std-node5.prod.rhcloud.com-2012-03-12


uid=6b44af7291524783ad6ed1bc1b55aed5
uid=8d3252d15512409c97f9d3b9167cc2bc
uid=e1f70ff7ee3a438a8d513ca953a3dc7c

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEUEARECAAYFAk9fJIsACgkQrlYvE4MpobNSPACYlgWtte2TcatDKgsfaVbz8WSY
XQCcCatA688MoFasF5sQUpQ6DSaNVKU=
=WEVQ
-END PGP SIGNATURE-

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] cpu: Add cpu definition for Intel Sandy Bridge cpu type

2012-03-13 Thread Peter Krempa

On 03/07/2012 02:30 PM, Peter Krempa wrote:

This patch adds support for the new tsc-deadline feature flag
and a new model to the supported model list describing the
Intel Sandy Bridge platform.
---
The Sandy Bridge processor model along with the tsc-deadline feature 
were just commited to qemu upstream as commits:


commit eaf3f0974ba48e3ebf76b331101ad333957432af
Author: Eduardo Habkost ehabk...@redhat.com
Date:   Tue Mar 6 15:11:30 2012 -0300

add tsc-deadline flag name to feature_ecx table

commit c34ea31416a9631b0a552afa08b99ec29cf44272
Author: Eduardo Habkost ehabk...@redhat.com
Date:   Tue Mar 6 15:11:31 2012 -0300

add SandyBridge CPU model

This patches add the definition of a SandyBridge CPU model.


Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Added support for AMD Bulldozer CPU

2012-03-13 Thread Martin Kletzander
AMD Bulldozer (or Opteron_G4 as called in QEMU) was added to the list
of cpu models, flags were taken from upstream qemu cpu specifications
and should be sorted by bit values (or first occurence in the feature
specification part of cpu_map.xml).

Based on QEMU upstream commit 885bb0369a4f0abe2c0185178f3cb347cb02cdf1.
---
 src/cpu/cpu_map.xml |   48 
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 6a6603b..8f80dd9 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -692,5 +692,53 @@
   feature name='misalignsse'/
   feature name='lahf_lm'/
 /model
+
+model name='Opteron_G4'
+  vendor name='AMD'/
+  feature name='fpu'/
+  feature name='de'/
+  feature name='pse'/
+  feature name='tsc'/
+  feature name='msr'/
+  feature name='pae'/
+  feature name='mce'/
+  feature name='cx8'/
+  feature name='apic'/
+  feature name='sep'/
+  feature name='mtrr'/
+  feature name='pge'/
+  feature name='mca'/
+  feature name='cmov'/
+  feature name='pat'/
+  feature name='pse36'/
+  feature name='clflush'/
+  feature name='mmx'/
+  feature name='fxsr'/
+  feature name='sse'/
+  feature name='sse2'/
+  feature name='pni'/
+  feature name='pclmuldq'/
+  feature name='ssse3'/
+  feature name='cx16'/
+  feature name='sse4.1'/
+  feature name='sse4.2'/
+  feature name='popcnt'/
+  feature name='aes'/
+  feature name='xsave'/
+  feature name='avx'/
+  feature name='syscall'/
+  feature name='nx'/
+  feature name='pdpe1gb'/
+  feature name='rdtscp'/
+  feature name='lm'/
+  feature name='lahf_lm'/
+  feature name='svm'/
+  feature name='abm'/
+  feature name='sse4a'/
+  feature name='misalignsse'/
+  feature name='3dnowprefetch'/
+  feature name='xop'/
+  feature name='fma4'/
+/model
   /arch
 /cpus
-- 
1.7.3.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Added support for AMD Bulldozer CPU

2012-03-13 Thread Peter Krempa

On 03/13/2012 12:35 PM, Martin Kletzander wrote:

AMD Bulldozer (or Opteron_G4 as called in QEMU) was added to the list
of cpu models, flags were taken from upstream qemu cpu specifications
and should be sorted by bit values (or first occurence in the feature
specification part of cpu_map.xml).

Based on QEMU upstream commit 885bb0369a4f0abe2c0185178f3cb347cb02cdf1.
---


ACK  pushed.

Sadly, qemu is not sticking to that ordering sometimes :(

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] Added support for AMD Bulldozer CPU

2012-03-13 Thread Martin Kletzander
On 03/13/2012 12:56 PM, Peter Krempa wrote:
 On 03/13/2012 12:35 PM, Martin Kletzander wrote:
 AMD Bulldozer (or Opteron_G4 as called in QEMU) was added to the list
 of cpu models, flags were taken from upstream qemu cpu specifications
 and should be sorted by bit values (or first occurence in the feature
 specification part of cpu_map.xml).

 Based on QEMU upstream commit 885bb0369a4f0abe2c0185178f3cb347cb02cdf1.
 ---
 
 ACK  pushed.
 

Thanks

 Sadly, qemu is not sticking to that ordering sometimes :(
 

I thought QEMU does, as well as /proc/cpuinfo etc. Anyway, remind me
when you have the Sandy bridge ACK'd, so I can do the whole cleanup
thing. Everything will be sorted then.

 Peter

Martin

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH RFC 8/8] add qemu cache mutex

2012-03-13 Thread Lee Schermerhorn
On Mon, 2012-03-12 at 10:16 -0400, Lee Schermerhorn wrote:
 On Mon, 2012-03-12 at 14:00 +0100, Michal Privoznik wrote:
  On 11.03.2012 19:56, Lee Schermerhorn wrote:
   Add a mutex for access to the qemu emulator cache.  Not clear that
   this is actually needed -- driver should be locked across calls [?].
   The patch can be dropped if not needed.
   ---
src/qemu/qemu_capabilities.c |   18 +-
src/qemu/qemu_capabilities.h |2 ++
src/qemu/qemu_driver.c   |3 +++
3 files changed, 22 insertions(+), 1 deletion(-)
   
   Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c
   ===
   --- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c
   +++ libvirt-0.9.10/src/qemu/qemu_capabilities.c
   @@ -27,6 +27,7 @@
#include memory.h
#include logging.h
#include virterror_internal.h
   +#include threads.h
#include util.h
#include virfile.h
#include nodeinfo.h
   @@ -180,6 +181,11 @@ enum qemuCapsProbes {
QEMU_PROBE_SIZE
};

   +/*
   + * Use static initializer for tests
   + */
   +static virMutex qemuEmulatorCacheMutex = { PTHREAD_MUTEX_INITIALIZER };
  
  This is not allowed in our code as we build with win32 threads which
  initialize mutexes completely different. Why do you want to initialize
  it here anyway ...
 
 
 Thanks.  I didn't know that.
 
 As the comment says, I added it for the internal tests.  It appeared to
 me that they don't go through the driver init code where I inserted the
 call to to the init function below.  The tests were hanging at the first
 attempt to acquire the mutex.  It could have been a defect in my patches
 at that time [that may still be there?].  When I inserted the static
 initializer, the tests passed.  That was back on the 0.8.8 ubuntu natty
 code base.  I'll pull it out and see if they pass w/o it, now that the
 tests all seem to pass otherwise.  They certainly pass w/o this patch
 applied, but they're all single threaded, right?

Update:  'make check' tests [qemuxml2argv] succeed with static
initializer removed.  Previous failures were apparently either an
artifact of the 0.8.8-1ubuntu6.x code base or a defect elsewhere in my
patches at the time.  I was still debugging then, trying to get tests to
pass.  Question whether the mutex is needed still stands, but since the
driver lock is RW, it probably is.  cache mutex could probably be made
RW as well, and only taken for W when filling/refreshing the cache.

I'm running more tests and will update the series when/if I hear that
it's worth doing so.

Regards,
Lee
 
 Bigger question is:  is the mutex actually needed at all?  I.e., can I
 assume that the driver is always locked -- in practice, not necessarily
 for the tests -- when the probes are called?
 
 Lee
 
   +
typedef struct _qemuEmulatorCache qemuEmulatorCache;
typedef qemuEmulatorCache* qemuEmulatorCachePtr;
struct _qemuEmulatorCache {
   @@ -206,9 +212,17 @@ qemuEmulatorCachedInfoGet(enum qemuCapsP
  const char *binary,
  const char *arch);

   +int
   +qemuCapsCacheInit(void)
   +{
   +return virMutexInit(qemuEmulatorCacheMutex);
   +}
   +
  
  if you have created this function?
static void
qemuEmulatorCachedInfoRelease(qemuEmulatorCachePtr emulator 
   ATTRIBUTE_UNUSED)
   -{ }
   +{
   +virMutexUnlock(qemuEmulatorCacheMutex);
   +}

 



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2] apparmor: QEMU bridge helper policy updates

2012-03-13 Thread Corey Bryant
This patch provides AppArmor policy updates for the QEMU bridge helper.
The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
capabilities to CAP_NET_ADMIN and adds a tap device to a network
bridge. For more details on the helper, please refer to:

http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03562.html

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
 examples/apparmor/libvirt-qemu |   22 +-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 10cdd36..c5a11b6 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -1,4 +1,4 @@
-# Last Modified: Mon Apr  5 15:11:27 2010
+# Last Modified: Fri Mar  9 14:43:22 2012
 
   #include abstractions/base
   #include abstractions/consoles
@@ -108,3 +108,23 @@
   /bin/dash rmix,
   /bin/dd rmix,
   /bin/cat rmix,
+
+  /usr/libexec/qemu-bridge-helper Cx,
+
+  # child profile for bridge helper process
+  profile /usr/libexec/qemu-bridge-helper {
+#include abstractions/base
+
+capability setuid,
+capability setgid,
+capability setpcap,
+capability net_admin,
+
+network inet stream,
+
+/dev/net/tun rw,
+/etc/qemu/** r,
+owner @{PROC}/*/status r,
+
+/usr/libexec/qemu-bridge-helper rmix,
+  }
-- 
1.7.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] apparmor: QEMU bridge helper policy updates

2012-03-13 Thread Corey Bryant



On 03/12/2012 05:07 PM, Corey Bryant wrote:
...

+ network inet stream,


I understood why net_admin was needed, but this one is less clear. Why
does qemu-bridge-helper need this?



Good question. I'm going to test without this and see if it's necessary.
I'm wondering if it's a subset of net_admin, and I may have added this
before I added net_admin.



I just submitted patch v2.  The only change is addition of the owner 
keyword (see below).


I tested without 'network inet stream' and got an AVC denial so I kept 
it in the policy.  We open an AF_INET/SOCK_STREAM socket in the helper, 
which I believe this maps to.


...

+ @{PROC}/*/status r,


Is it possible to use this instead:
owner @{PROC}/*/status r,



I would imagine this is ok to update with owner. I'll test it out and
submit a v2 if there aren't any issues.




--
Regards,
Corey

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] vmware: detect when a domain was shut down from the inside

2012-03-13 Thread Jean-Baptiste Rouault
On Friday 03 February 2012 09:42:29 Matthias Bolte wrote: 
 You changed all lifecycle functions not to rely on possible stale
 information, but you missed to update the cached state information in
 the virDomainObj list resulting in virsh list giving wrong output and
 possibly listing VMs as active that aren't active anymore.
 
 Maybe you should replace vmwareGetVMStatus with vmwareUpdateVMStatus
 that updates the cached information in a virDomainObj and call it in
 all places that read information from a virDomainObj.
 
 A more general question: is this driver supposed to work properly when
 someone uses vmrun to alter a VMs behind libvirt's back? If that's the
 case then maybe this driver should not cache anything at all and work
 like the vSphere, Hyper-V or VirtualBox driver. They always query the
 hypervisor for information and cache nothing.
 
 I'm not sure what's the best approach here.

Hi,

The problem here is that contrary to vSphere, Hyper-V or VirtualBox, I have no 
way to get a list of defined domains as vmrun only lists running domains.
So I think I'll do as you suggested and go for a vmwareUpdateVMStatus
function.

Regards,
Jean-Baptiste

-- 
Jean-Baptiste ROUAULT
Ingénieur RD - diateam : Architectes de l'information
Phone : +33 (0)9 53 16 02 70 Fax : +33 (0)2 98 050 051

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/4] storage: Implement jobs for storage driver

2012-03-13 Thread Michal Privoznik
Simply, when we are about to take an action which might take ages,
like allocating new volumes, wiping, etc. increment a counter of
jobs in pool object and unlock it. We don't want to hold the pool
locked during long term actions.
---
 src/conf/storage_conf.c  |   12 ++
 src/conf/storage_conf.h  |   22 +++-
 src/libvirt_private.syms |1 +
 src/storage/storage_driver.c |  284 +++---
 4 files changed, 246 insertions(+), 73 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index bdf6218..e378ceb 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -251,6 +251,8 @@ virStorageVolDefFree(virStorageVolDefPtr def) {
 if (!def)
 return;
 
+virMutexDestroy(def-lock);
+ignore_value(virCondDestroy(def-job.cond));
 VIR_FREE(def-name);
 VIR_FREE(def-key);
 
@@ -978,6 +980,16 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
 return NULL;
 }
 
+if (virMutexInit(ret-lock)  0) {
+virReportSystemError(errno, %s, _(Cannot init mutex));
+goto cleanup;
+}
+
+if (virCondInit(ret-job.cond)  0) {
+virReportSystemError(errno, %s, _(Cannot init cond));
+goto cleanup;
+}
+
 ret-name = virXPathString(string(./name), ctxt);
 if (ret-name == NULL) {
 virStorageReportError(VIR_ERR_XML_ERROR,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 1ef9295..481c806 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -83,6 +83,25 @@ struct _virStorageVolTarget {
 };
 
 
+enum virStorageVolJob {
+VIR_STORAGE_VOL_JOB_NONE = 0, /* no job */
+VIR_STORAGE_VOL_JOB_BUILD,
+VIR_STORAGE_VOL_JOB_DELETE,
+VIR_STORAGE_VOL_JOB_REFRESH,
+VIR_STORAGE_VOL_JOB_WIPE,
+VIR_STORAGE_VOL_JOB_RESIZE,
+VIR_STORAGE_VOL_JOB_DOWNLOAD,
+VIR_STORAGE_VOL_JOB_UPLOAD,
+
+VIR_STORAGE_VOL_JOB_LAST
+};
+
+struct virStorageVolJobObj {
+virCond cond;
+enum virStorageVolJob active;
+unsigned long long start;
+};
+
 typedef struct _virStorageVolDef virStorageVolDef;
 typedef virStorageVolDef *virStorageVolDefPtr;
 struct _virStorageVolDef {
@@ -90,7 +109,8 @@ struct _virStorageVolDef {
 char *key;
 int type; /* virStorageVolType enum */
 
-unsigned int building;
+virMutex lock;
+struct virStorageVolJobObj job;
 
 unsigned long long allocation; /* bytes */
 unsigned long long capacity; /* bytes */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1f55f5d..fef9d5a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,6 +565,7 @@ virFDStreamOpen;
 virFDStreamConnectUNIX;
 virFDStreamOpenFile;
 virFDStreamCreateFile;
+virFDStreamSetInternalCloseCb;
 
 
 # hash.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 66811ce..7f3dfcd 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -48,6 +48,7 @@
 #include virfile.h
 #include fdstream.h
 #include configmake.h
+#include virtime.h
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
@@ -65,6 +66,111 @@ static void storageDriverUnlock(virStorageDriverStatePtr 
driver)
 }
 
 static void
+storageResetJob(virStorageVolDefPtr vol)
+{
+struct virStorageVolJobObj *job = vol-job;
+
+job-active = VIR_STORAGE_VOL_JOB_NONE;
+job-start = 0;
+}
+
+/* wait max. 30 seconds for job ackquire */
+#define STORAGE_JOB_WAIT_TIME (1000ull * 30)
+
+static int
+storageBeginJobInternal(virStorageDriverStatePtr driver ATTRIBUTE_UNUSED,
+virStoragePoolObjPtr pool,
+bool pool_locked,
+virStorageVolDefPtr vol,
+enum virStorageVolJob job)
+{
+unsigned long long now;
+unsigned long long then;
+
+if (virTimeMillisNow(now)  0)
+return -1;
+
+then = now + STORAGE_JOB_WAIT_TIME;
+
+while (vol-job.active != VIR_STORAGE_VOL_JOB_NONE) {
+if (virCondWaitUntil(vol-job.cond, vol-lock, then)  0)
+goto error;
+}
+
+VIR_DEBUG(Starting job %d, job);
+storageResetJob(vol);
+vol-job.active = job;
+vol-job.start = now;
+
+if (pool_locked) {
+pool-asyncjobs++;
+virStoragePoolObjUnlock(pool);
+}
+
+return 0;
+
+error:
+if (errno == ETIMEDOUT)
+virStorageReportError(VIR_ERR_OPERATION_TIMEOUT, %s,
+  _(cannot acquire state change lock));
+else
+virReportSystemError(errno, %s,
+ _(cannot acquire job mutex));
+if (pool_locked)
+virStoragePoolObjLock(pool);
+return -1;
+}
+
+static int
+storageBeginJob(virStorageDriverStatePtr driver,
+virStorageVolDefPtr vol,
+enum virStorageVolJob job)
+{
+return storageBeginJobInternal(driver, NULL, false, vol, job);
+}
+
+static int
+storageBeginJobWithPool(virStorageDriverStatePtr driver,
+

[libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob

2012-03-13 Thread Michal Privoznik
This API can be used to terminate long running jobs
on a volume like its building, resizing, wiping.
Moreover, like virDomainAbortJob() calling this API
will block until job has either completed or aborted.
---
 include/libvirt/libvirt.h.in |3 ++
 src/driver.h |5 
 src/libvirt.c|   49 ++
 src/libvirt_public.syms  |1 +
 src/remote/remote_driver.c   |1 +
 src/remote/remote_protocol.x |8 ++-
 src/remote_protocol-structs  |5 
 7 files changed, 71 insertions(+), 1 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 7d41642..77ec3f0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2513,6 +2513,9 @@ int virStorageVolResize 
(virStorageVolPtr vol,
  unsigned long long 
capacity,
  unsigned int flags);
 
+int virStorageVolAbortJob   (virStorageVolPtr vol,
+ unsigned int flags);
+
 
 /**
  * virKeycodeSet:
diff --git a/src/driver.h b/src/driver.h
index 03d249b..7845b06 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1314,6 +1314,10 @@ typedef int
unsigned int flags);
 
 typedef int
+(*virDrvStorageVolAbortJob) (virStorageVolPtr vol,
+ unsigned int flags);
+
+typedef int
 (*virDrvStoragePoolIsActive)(virStoragePoolPtr pool);
 typedef int
 (*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool);
@@ -1377,6 +1381,7 @@ struct _virStorageDriver {
 virDrvStorageVolResize volResize;
 virDrvStoragePoolIsActive   poolIsActive;
 virDrvStoragePoolIsPersistent   poolIsPersistent;
+virDrvStorageVolAbortJobvolAbortJob;
 };
 
 # ifdef WITH_LIBVIRTD
diff --git a/src/libvirt.c b/src/libvirt.c
index e916aa0..8ce3234 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13343,6 +13343,55 @@ error:
 }
 
 /**
+ * virStorageVolAbortJob:
+ * @vol: pointer to storage volume
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Requests that the current background job be aborted at the soonest
+ * opportunity. This will block until the job has either completed,
+ * or aborted.
+ *
+ * Returns:  0 in case of success
+ *  -1 otherwise
+ */
+int
+virStorageVolAbortJob(virStorageVolPtr vol,
+  unsigned int flags)
+{
+virConnectPtr conn;
+VIR_DEBUG(vol=%p flags=%x, vol, flags);
+
+virResetLastError();
+
+if (!VIR_IS_STORAGE_VOL(vol)) {
+virLibStorageVolError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+
+conn = vol-conn;
+
+if (conn-flags  VIR_CONNECT_RO) {
+   virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+   goto error;
+}
+
+if (conn-storageDriver  conn-storageDriver-volAbortJob) {
+int ret;
+ret = conn-storageDriver-volAbortJob(vol, flags);
+if (ret  0)
+goto error;
+return ret;
+}
+
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(vol-conn);
+return -1;
+}
+
+/**
  * virNodeNumOfDevices:
  * @conn: pointer to the hypervisor connection
  * @cap: capability name
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 46c13fb..cd3e2a6 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -532,6 +532,7 @@ LIBVIRT_0.9.10 {
 LIBVIRT_0.9.11 {
 global:
 virDomainPMWakeup;
+virStorageVolAbortJob;
 } LIBVIRT_0.9.10;
 
 #  define new API here using predicted next version number 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 031167a..3534ac0 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5013,6 +5013,7 @@ static virStorageDriver storage_driver = {
 .volResize = remoteStorageVolResize, /* 0.9.10 */
 .poolIsActive = remoteStoragePoolIsActive, /* 0.7.3 */
 .poolIsPersistent = remoteStoragePoolIsPersistent, /* 0.7.3 */
+.volAbortJob = remoteStorageVolAbortJob, /* 0.9.11 */
 };
 
 static virSecretDriver secret_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 4d845e7..014eade 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1754,6 +1754,11 @@ struct remote_storage_vol_resize_args {
 unsigned int flags;
 };
 
+struct remote_storage_vol_abort_job_args {
+remote_nonnull_storage_vol vol;
+unsigned int flags;
+};
+
 /* Node driver calls: */
 
 struct remote_node_num_of_devices_args {
@@ -2765,7 +2770,8 @@ enum remote_procedure {
 REMOTE_PROC_DOMAIN_SET_METADATA = 264, /* autogen autogen */
 REMOTE_PROC_DOMAIN_GET_METADATA = 265, /* autogen autogen */
 

[libvirt] [PATCH 0/4] Introduce jobs for storage driver

2012-03-13 Thread Michal Privoznik
Disk operations can take ages to finish. Therefore users
might want to abort such job, e.g. because host is going
down for maintenance. This patch set is trying to allow
this kind of behaviour. The inspiration was taken from
qemu driver.

How it works:
An API that is known to run for a long time, e.g. volume
allocation, wiping, have to find a pool which volume
belongs to. If the pool was found it is locked. During
this time the storage driver is locked. Now the API is
preparing for taking the operation. Ideally, it would be
sufficient to have only volume locked, but it requires
far more changes to code.

Just before the API is about to take the long term action,
storageBeginJob[WithPool] is called. This wait on a condition
specific for a volume. Similar to QEMU driver, wait is
limited in time. Timeout is set to 30 seconds.
Upon successful job acquire, a counter in the pool 
is incremented so the pool is prevented from destroy,
undef; Then the pool is unlocked.

After API finishes it's work, it calls storageEndJob[WithPool]
which reset job structure in the volume, re-locks
the pool, decrement the counter and broadcasts on
the condition.

Meanwhile, if the long term action is done internally,
it should be done in a cycle so we can check if job abort
wasn't signalized. If it is done by external program,
we can virCommandAbort() it.

Several improvements can be made, but I think this is
good for start.

NB, I've implemented job aborting for the both APIs using
streams, however wasn't very successful with aborting
a stream from the daemon site.

Michal Privoznik (4):
  storage: Introduce virStorageVolAbortJob
  virsh: Expose virStorageVolAbortJob
  storage: Implement jobs for storage driver
  storage: Implement virStorageVolAbortJob

 include/libvirt/libvirt.h.in |3 +
 src/conf/storage_conf.c  |   12 ++
 src/conf/storage_conf.h  |   29 +++-
 src/driver.h |5 +
 src/libvirt.c|   49 +
 src/libvirt_private.syms |4 +
 src/libvirt_public.syms  |1 +
 src/remote/remote_driver.c   |1 +
 src/remote/remote_protocol.x |8 +-
 src/remote_protocol-structs  |5 +
 src/storage/storage_backend.c|   53 +++---
 src/storage/storage_backend_fs.c |8 +-
 src/storage/storage_driver.c |  408 ++
 src/storage/storage_driver.h |7 +
 tools/virsh.c|   39 
 15 files changed, 516 insertions(+), 116 deletions(-)

-- 
1.7.8.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/4] virsh: Expose virStorageVolAbortJob

2012-03-13 Thread Michal Privoznik
via new virsh command 'vol-jobabort'. Currently, it accepts
only volume specification as argument.
---
 tools/virsh.c |   39 +++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 630b77f..00668ff 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12497,6 +12497,44 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
 return true;
 }
 
+/*
+ * vol-jobabort command
+ */
+static const vshCmdInfo info_vol_jobabort[] = {
+{help, N_(abort active volume job)},
+{desc, N_(Aborts the currently running volume job)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_vol_jobabort[] = {
+{vol, VSH_OT_DATA, VSH_OFLAG_REQ, N_(volume name or key)},
+{pool, VSH_OT_STRING, 0, N_(pool name or uuid)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdVolJobAbort(vshControl *ctl, const vshCmd *cmd)
+{
+virStorageVolPtr vol;
+bool ret = false;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return false;
+
+if (!(vol = vshCommandOptVol(ctl, cmd, vol, pool, NULL))) {
+return false;
+}
+
+if (virStorageVolAbortJob(vol, 0)  0)
+goto cleanup;
+
+vshPrint(ctl, _(Job successfully aborted));
+ret = true;
+
+cleanup:
+virStorageVolFree(vol);
+return ret;
+}
 
 /*
  * secret-define command
@@ -17239,6 +17277,7 @@ static const vshCmdDef storageVolCmds[] = {
 {vol-download, cmdVolDownload, opts_vol_download, info_vol_download, 0},
 {vol-dumpxml, cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml, 0},
 {vol-info, cmdVolInfo, opts_vol_info, info_vol_info, 0},
+{vol-jobabort, cmdVolJobAbort, opts_vol_jobabort, info_vol_jobabort, 0},
 {vol-key, cmdVolKey, opts_vol_key, info_vol_key, 0},
 {vol-list, cmdVolList, opts_vol_list, info_vol_list, 0},
 {vol-name, cmdVolName, opts_vol_name, info_vol_name, 0},
-- 
1.7.8.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 4/4] storage: Implement virStorageVolAbortJob

2012-03-13 Thread Michal Privoznik
This implies breaking up some jobs into cycles during which
we check for job abortion request. The virStorageVolAbortJob
API will then just set request and wait until job is released.
If a job was, however, interrupted it should fail with
VIR_ERR_OPERATION_ABORTED error.
---
 src/conf/storage_conf.h  |7 ++
 src/libvirt_private.syms |3 +
 src/storage/storage_backend.c|   53 +---
 src/storage/storage_backend_fs.c |8 ++-
 src/storage/storage_driver.c |  128 -
 src/storage/storage_driver.h |7 ++
 6 files changed, 162 insertions(+), 44 deletions(-)

diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 481c806..ee25e34 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -28,6 +28,7 @@
 # include util.h
 # include storage_encryption_conf.h
 # include threads.h
+# include command.h
 
 # include libxml/tree.h
 
@@ -100,6 +101,12 @@ struct virStorageVolJobObj {
 virCond cond;
 enum virStorageVolJob active;
 unsigned long long start;
+
+bool abort_requested;   /* abort was requested */
+virCommandPtr cmd;  /* if we are running external program,
+   store pointer here too, so we can
+   virCommandAbort it if necessary */
+virStreamPtr stream;/* stream to abort */
 };
 
 typedef struct _virStorageVolDef virStorageVolDef;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fef9d5a..cec7a94 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1017,6 +1017,9 @@ virStorageVolDefParseFile;
 virStorageVolDefParseNode;
 virStorageVolDefParseString;
 
+# storage_driver.h
+virStorageVolJobSetCommand;
+virStorageVolJobSetStream;
 
 # storage_encryption_conf.h
 virStorageEncryptionFormat;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index caac2f8..2e82c03 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -53,6 +53,7 @@
 #include internal.h
 #include secret_conf.h
 #include uuid.h
+#include storage_driver.h
 #include storage_file.h
 #include storage_backend.h
 #include logging.h
@@ -106,8 +107,6 @@ static virStorageBackendPtr backends[] = {
 NULL
 };
 
-static int track_allocation_progress = 0;
-
 enum {
 TOOL_QEMU_IMG,
 TOOL_KVM_IMG,
@@ -202,6 +201,15 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 goto cleanup;
 
 }
+
+if (vol-job.abort_requested ||
+inputvol-job.abort_requested) {
+virStorageReportError(VIR_ERR_OPERATION_ABORTED, %s,
+  _(Job abort requested));
+ret = -1;
+goto cleanup;
+}
+
 } while ((amtleft -= interval)  0);
 }
 
@@ -325,34 +333,31 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 }
 
 if (remain) {
-if (track_allocation_progress) {
+while (remain) {
+/* Allocate in chunks of 512MiB: big-enough chunk
+ * size and takes approx. 9s on ext3. A progress
+ * update every 9s is a fair-enough trade-off
+ */
+unsigned long long bytes = 512 * 1024 * 1024;
 
-while (remain) {
-/* Allocate in chunks of 512MiB: big-enough chunk
- * size and takes approx. 9s on ext3. A progress
- * update every 9s is a fair-enough trade-off
- */
-unsigned long long bytes = 512 * 1024 * 1024;
-
-if (bytes  remain)
-bytes = remain;
-if (safezero(fd, vol-allocation - remain, bytes)  0) {
-ret = -errno;
-virReportSystemError(errno, _(cannot fill file '%s'),
- vol-target.path);
-goto cleanup;
-}
-remain -= bytes;
-}
-} else { /* No progress bars to be shown */
-if (safezero(fd, 0, remain)  0) {
+if (bytes  remain)
+bytes = remain;
+if (safezero(fd, vol-allocation - remain, bytes)  0) {
 ret = -errno;
 virReportSystemError(errno, _(cannot fill file '%s'),
  vol-target.path);
 goto cleanup;
 }
-}
+remain -= bytes;
 
+if (vol-job.abort_requested) {
+virStorageReportError(VIR_ERR_OPERATION_ABORTED,
+  %s,
+  _(Job abort requested));
+ret = -1;
+goto cleanup;
+}
+}
 }
 
 if (fsync(fd)  0) {
@@ -782,6 +787,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
 goto cleanup;
 
 cmd = virCommandNew(create_tool);
+virStorageVolJobSetCommand(vol, 

Re: [libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob

2012-03-13 Thread Daniel P. Berrange
On Tue, Mar 13, 2012 at 03:35:29PM +0100, Michal Privoznik wrote:
 This API can be used to terminate long running jobs
 on a volume like its building, resizing, wiping.
 Moreover, like virDomainAbortJob() calling this API
 will block until job has either completed or aborted.
 ---
  include/libvirt/libvirt.h.in |3 ++
  src/driver.h |5 
  src/libvirt.c|   49 
 ++
  src/libvirt_public.syms  |1 +
  src/remote/remote_driver.c   |1 +
  src/remote/remote_protocol.x |8 ++-
  src/remote_protocol-structs  |5 
  7 files changed, 71 insertions(+), 1 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 7d41642..77ec3f0 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -2513,6 +2513,9 @@ int virStorageVolResize 
 (virStorageVolPtr vol,
   unsigned long long 
 capacity,
   unsigned int flags);
  
 +int virStorageVolAbortJob   (virStorageVolPtr 
 vol,
 + unsigned int flags);
 +

No,  virStorageVolGetJobInfo()  API to go with it ?   IMHO we should have
both, so we mirror the virDomain job API design.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [Qemu-devel] Modern CPU models cannot be used with libvirt

2012-03-13 Thread Eduardo Habkost
So, trying to summarize what was discussed in the call:

On Mon, Mar 12, 2012 at 10:08:10AM -0300, Eduardo Habkost wrote:
  Let's say we moved CPU definitions to /usr/share/qemu/cpu-models.xml.
  
  Obviously, we'd want a command line option to be able to change that
  location so we'd introduce -cpu-models PATH.
  
  But we want all of our command line options to be settable by the
  global configuration file so we would have a cpu-model=PATH to the
  configuration file.
  
  But why hard code a path when we can just set the default path in the
  configuration file so let's avoid hard coding and just put
  cpu-models=/usr/share/qemu/cpu-models.xml in the default
  configuration file.
 
 We wouldn't do the above.
 
 -nodefconfig should disable the loading of files on /etc, but it
 shouldn't disable loading internal non-configurable data that we just
 happened to choose to store outside the qemu binary because it makes
 development easier.

The statement above is the one not fulfilled by the compromise solution:
-nodefconfig would really disable the loading of files on /usr/share.

 
 Really, the requirement of a default configuration file is a problem
 by itself. Qemu should not require a default configuration file to work,
 and it shouldn't require users to copy the default configuration file to
 change options from the default.

The statement above is only partly true. The default configuration file
would be still needed, but if defaults are stored on /usr/share, I will
be happy with it.

My main problem was with the need to _copy_ or edit a non-trivial
default config file. If the not-often-edited defaults/templates are
easily found on /usr/share to be used with -readconfig, I will be happy
with this solution, even if -nodefconfig disable the files on
/usr/share.

 
 Doing this would make it impossible to deploy fixes to users if we evern
 find out that the default configuration file had a serious bug. What if
 a bug in our default configuration file has a serious security
 implication?

The answer to this is: if the broken templates/defaults are on
/usr/share, it would be easy to deploy the fix.

So, the compromise solution is:

- We can move some configuration data (especially defaults/templates)
  to /usr/share (machine-types and CPU models could go there). This
  way we can easily deploy fixes to the defaults, if necessary.
- To reuse Qemu models, or machine-types, and not define everything from
  scratch, libvirt will have to use something like:
  -nodefconfig -readconfig /usr/share/qemu/cpu-models-x86.conf


(the item below is not something discussed on the call, just something I
want to add)

To make this work better, we can allow users (humans or machines) to
extend CPU models on the config file, instead of having to define
everything from scratch. So, on /etc (or on a libvirt-generated config)
we could have something like:

=
[cpu]
base_cpudef = Nehalem
add_features = vmx
=

Then, as long as /usr/share/cpu-models-x86.conf is loaded, the user will
be able to reuse the Nehalem CPU model provided by Qemu.

 
  
  But now when libvirt uses -nodefconfig, those models go away.
  -nodefconfig means start QEMU in the most minimal state possible.
  You get what you pay for if you use it.
  
  We'll have the same problem with machine configuration files.  At
  some point in time, -nodefconfig will make machine models disappear.
 
 It shouldn't. Machine-types are defaults to be used as base, they are
 not user-provided configuration. And the fact that we decided to store
 some data outside of the Qemu binary is orthogonal the design decisions
 in the Qemu command-line and configuration interface.

So, this problem is solved if the defaults are easily found on
/usr/share.

We still have the backwards compatibility problem for pc-1.0, pc-1.1,
and so on. But that can be discussed later, when we actually move
machine-types to somewhere outside .c files.

 
 As I said previously, requiring generation of opaque config files (and
 copy the default config file and change it is included on my
 definition of generation of opaque config files) is poor design, IMO.
 I bet this even has an entry in some design anti-pattern catalog
 somewhere.

This problem is also solved if the defaults are deployed on /usr/share
and just reused/included by the config files on /etc.

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/4] storage: Introduce virStorageVolAbortJob

2012-03-13 Thread Michal Privoznik
On 13.03.2012 15:48, Daniel P. Berrange wrote:
 On Tue, Mar 13, 2012 at 03:35:29PM +0100, Michal Privoznik wrote:
 This API can be used to terminate long running jobs
 on a volume like its building, resizing, wiping.
 Moreover, like virDomainAbortJob() calling this API
 will block until job has either completed or aborted.
 ---
  include/libvirt/libvirt.h.in |3 ++
  src/driver.h |5 
  src/libvirt.c|   49 
 ++
  src/libvirt_public.syms  |1 +
  src/remote/remote_driver.c   |1 +
  src/remote/remote_protocol.x |8 ++-
  src/remote_protocol-structs  |5 
  7 files changed, 71 insertions(+), 1 deletions(-)

 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 7d41642..77ec3f0 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -2513,6 +2513,9 @@ int virStorageVolResize
  (virStorageVolPtr vol,
   unsigned long long 
 capacity,
   unsigned int 
 flags);
  
 +int virStorageVolAbortJob   (virStorageVolPtr 
 vol,
 + unsigned int 
 flags);
 +
 
 No,  virStorageVolGetJobInfo()  API to go with it ?   IMHO we should have
 both, so we mirror the virDomain job API design.
 
 Regards,
 Daniel

yeah, virStorageVolGetJobInfo() is one of the improvements I'm
mentioning in cover letter. But I've decided to not implement it for now
as another huge bunch of code would have to be rewritten make this patch
set unbearable big. But if it is a show stopper I can rewrite and post v2.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Christophe Fergeau
Hey,

On Tue, Mar 13, 2012 at 01:22:09AM +0100, Marc-André Lureau wrote:
 On Mon, Mar 12, 2012 at 5:56 PM, Christophe Fergeau cferg...@redhat.com 
 wrote:
  Ping for this patch and for 3/3 ?
 
 ./test-domain-create gives:
 
 channel type=spicevmc
   target type=channel-target-virtio/
 /channel
 
 Where we expect this:
 
 channel type='spicevmc'
   target type='virtio' name='com.redhat.spice.0'/
   address type='virtio-serial' controller='0' bus='0' port='1'/
 /channel

The address element is optional, see
http://libvirt.org/formatdomain.html#elementCharChannel The optional
address element can tie the channel to a particular type='virtio-serial'
controller.
It seems libvirt will still do the right thing if it's omitted:
http://libvirt.org/formatdomain.html#elementsAddress
I tend to only add API in libvirt-gconfig when there's a need for it, but I
can look into adding API to set the address element if you think that's
needed now.

Similarly, the name attribute is optional, and defaults to
'com.redhat.spice.0'. It can be set with
vir_config_domain_channel_set_target_name

While looking at this, I've found that I need to squash this in this patch:

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.h 
b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.h
index 6b2ab20..8bbe634 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.h
@@ -62,9 +62,6 @@ GType 
gvir_config_domain_chardev_source_spicevmc_get_type(void);
 GVirConfigDomainChardevSourceSpiceVmc 
*gvir_config_domain_chardev_source_spicevmc_new(void);
 GVirConfigDomainChardevSourceSpiceVmc 
*gvir_config_domain_chardev_source_spicevmc_new_from_xml(const gchar *xml,
   
GError **error);
-void 
gvir_config_domain_source_spicevmc_set_path(GVirConfigDomainChardevSourceSpiceVmc
 *spicevmc,
- const char *path);
-
 G_END_DECLS
 
 #endif /* __LIBVIRT_GCONFIG_DOMAIN_CHARDEV_SOURCE_SPICE_VMC_H__ */

Christophe

 
 
  Christophe
 
  On Tue, Mar 06, 2012 at 05:38:33PM +0100, Christophe Fergeau wrote:
  This is needed to be able to add the SPICE agent channels
  ---
   libvirt-gconfig/Makefile.am                        |    2 +
   ...ibvirt-gconfig-domain-chardev-source-spicevmc.c |   80 
  
   ...ibvirt-gconfig-domain-chardev-source-spicevmc.h |   70 
  +
   libvirt-gconfig/libvirt-gconfig.h                  |    1 +
   libvirt-gconfig/libvirt-gconfig.sym                |    4 +
   libvirt-gconfig/tests/test-domain-create.c         |   14 
   6 files changed, 171 insertions(+), 0 deletions(-)
   create mode 100644 
  libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.c
   create mode 100644 
  libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.h
 
  diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
  index 03a5507..d9e87b5 100644
  --- a/libvirt-gconfig/Makefile.am
  +++ b/libvirt-gconfig/Makefile.am
  @@ -17,6 +17,7 @@ GCONFIG_HEADER_FILES = \
                        libvirt-gconfig-domain-chardev.h \
                        libvirt-gconfig-domain-chardev-source.h \
                        libvirt-gconfig-domain-chardev-source-pty.h \
  +                     libvirt-gconfig-domain-chardev-source-spicevmc.h \
                        libvirt-gconfig-domain-clock.h \
                        libvirt-gconfig-domain-console.h \
                        libvirt-gconfig-domain-device.h \
  @@ -68,6 +69,7 @@ GCONFIG_SOURCE_FILES = \
                        libvirt-gconfig-domain-chardev.c \
                        libvirt-gconfig-domain-chardev-source.c \
                        libvirt-gconfig-domain-chardev-source-pty.c \
  +                     libvirt-gconfig-domain-chardev-source-spicevmc.c \
                        libvirt-gconfig-domain-clock.c \
                        libvirt-gconfig-domain-console.c \
                        libvirt-gconfig-domain-device.c \
  diff --git 
  a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.c 
  b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.c
  new file mode 100644
  index 000..22eabf5
  --- /dev/null
  +++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.c
  @@ -0,0 +1,80 @@
  +/*
  + * libvirt-gconfig-domain-chardev-source-spicevmc.c: libvirt domain 
  chardev spicevmc configuration
  + *
  + * Copyright (C) 2012 Red Hat, Inc.
  + *
  + * This library is free software; you can redistribute it and/or
  + * modify it under the terms of the GNU Lesser General Public
  + * License as published by the Free Software Foundation; either
  + * version 2.1 of the License, or (at your option) any later version.
  + *
  + * This library is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without 

Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Marc-André Lureau
On Tue, Mar 13, 2012 at 4:15 PM, Christophe Fergeau cferg...@redhat.com wrote:
       target type=channel-target-virtio/

That's what I was mainly looking at, and I wish the test would cover a
more complex and needed case, just to be sure.

-- 
Marc-André Lureau

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] qemu: Copy console definition from serial

2012-03-13 Thread Jan Kiszka
On 2012-03-09 19:11, Jan Kiszka wrote:
 On 2012-03-09 19:02, Daniel P. Berrange wrote:
 On Fri, Mar 09, 2012 at 06:58:06PM +0100, Jan Kiszka wrote:
 On 2012-03-09 18:53, Daniel P. Berrange wrote:
 On Fri, Mar 09, 2012 at 06:48:42PM +0100, Jan Kiszka wrote:
 On 2011-11-16 14:14, Michal Privoznik wrote:
 Now, when we support multiple consoles per domain,
 the vm-def-console[0] can still remain an alias
 for vm-def-serial[0]; However, we need to copy
 it's source definition as well otherwise we'll regress
 on virDomainOpenConsole.

 ...

 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
 index 2882ef8..e0b1824 100644
 --- a/src/qemu/qemu_process.c
 +++ b/src/qemu/qemu_process.c
 @@ -1163,11 +1163,22 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm,
  
  for (i = 0 ; i  vm-def-nconsoles ; i++) {
  virDomainChrDefPtr chr = vm-def-consoles[i];
 -if (chr-source.type == VIR_DOMAIN_CHR_TYPE_PTY 
 -chr-targetType == 
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
 -if ((ret = qemuProcessExtractTTYPath(output, offset,
 - 
 chr-source.data.file.path)) != 0)
 +/* For historical reasons, console[0] can be just an alias
 + * for serial[0]; That's why we need to update it as well */
 +if (i == 0  vm-def-nserials 
 +chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
 +chr-targetType == 
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
 +if ((ret = virDomainChrSourceDefCopy(chr-source,
 + 
 ((vm-def-serials[0])-source))) != 0)
  return ret;
 +chr-source.type = VIR_DOMAIN_CHR_TYPE_PTY;

 This unconditional setting of TYPE_PTY breaks serial on stdio (we use
 this to easily fold guest into host logs). Can you explain why the
 copied source.type of serial[0] is not always correct? Or are we already
 in the wrong branch for a

 serial type='stdio'/serial

 configuration?

 Yeah I think this is a bug. The first serial element should match the
 first console exactly, with targetType==serial. We shouldn't be
 forcing it to type=pty

 So, if vm-def-serials[0])-source.type != VIR_DOMAIN_CHR_TYPE_PTY, we
 should skip this fixup branch?

 No, we should do the fix for all types. serial[0] and console[0] should
 be identical, unless the console[0] targetType != TARGET_TYPE_SERIAL
 
 Well, libvirt is alien to me, and at this point I'm lost in fixups. :)
 Do you mean simply dropping the source.type = TYPE_PTY line?
 

Ping?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Daniel P. Berrange
On Tue, Mar 13, 2012 at 04:18:21PM +0100, Marc-André Lureau wrote:
 On Tue, Mar 13, 2012 at 4:15 PM, Christophe Fergeau cferg...@redhat.com 
 wrote:
        target type=channel-target-virtio/
 
 That's what I was mainly looking at, and I wish the test would cover a
 more complex and needed case, just to be sure.

Yes, that XML does look a bit wrong.


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Copy console definition from serial

2012-03-13 Thread Daniel P. Berrange
On Fri, Mar 09, 2012 at 07:11:05PM +0100, Jan Kiszka wrote:
 On 2012-03-09 19:02, Daniel P. Berrange wrote:
  On Fri, Mar 09, 2012 at 06:58:06PM +0100, Jan Kiszka wrote:
  On 2012-03-09 18:53, Daniel P. Berrange wrote:
  On Fri, Mar 09, 2012 at 06:48:42PM +0100, Jan Kiszka wrote:
  On 2011-11-16 14:14, Michal Privoznik wrote:
  Now, when we support multiple consoles per domain,
  the vm-def-console[0] can still remain an alias
  for vm-def-serial[0]; However, we need to copy
  it's source definition as well otherwise we'll regress
  on virDomainOpenConsole.
 
  ...
 
  diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
  index 2882ef8..e0b1824 100644
  --- a/src/qemu/qemu_process.c
  +++ b/src/qemu/qemu_process.c
  @@ -1163,11 +1163,22 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr 
  vm,
   
   for (i = 0 ; i  vm-def-nconsoles ; i++) {
   virDomainChrDefPtr chr = vm-def-consoles[i];
  -if (chr-source.type == VIR_DOMAIN_CHR_TYPE_PTY 
  -chr-targetType == 
  VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
  -if ((ret = qemuProcessExtractTTYPath(output, offset,
  - 
  chr-source.data.file.path)) != 0)
  +/* For historical reasons, console[0] can be just an alias
  + * for serial[0]; That's why we need to update it as well */
  +if (i == 0  vm-def-nserials 
  +chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE 
  +chr-targetType == 
  VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
  +if ((ret = virDomainChrSourceDefCopy(chr-source,
  + 
  ((vm-def-serials[0])-source))) != 0)
   return ret;
  +chr-source.type = VIR_DOMAIN_CHR_TYPE_PTY;
 
  This unconditional setting of TYPE_PTY breaks serial on stdio (we use
  this to easily fold guest into host logs). Can you explain why the
  copied source.type of serial[0] is not always correct? Or are we already
  in the wrong branch for a
 
  serial type='stdio'/serial
 
  configuration?
 
  Yeah I think this is a bug. The first serial element should match the
  first console exactly, with targetType==serial. We shouldn't be
  forcing it to type=pty
 
  So, if vm-def-serials[0])-source.type != VIR_DOMAIN_CHR_TYPE_PTY, we
  should skip this fixup branch?
  
  No, we should do the fix for all types. serial[0] and console[0] should
  be identical, unless the console[0] targetType != TARGET_TYPE_SERIAL
 
 Well, libvirt is alien to me, and at this point I'm lost in fixups. :)
 Do you mean simply dropping the source.type = TYPE_PTY line?

Yes, I think that line can just be removed.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Daniel P. Berrange
On Tue, Mar 13, 2012 at 04:15:59PM +0100, Christophe Fergeau wrote:
 Hey,
 
 On Tue, Mar 13, 2012 at 01:22:09AM +0100, Marc-André Lureau wrote:
  On Mon, Mar 12, 2012 at 5:56 PM, Christophe Fergeau cferg...@redhat.com 
  wrote:
   Ping for this patch and for 3/3 ?
  
  ./test-domain-create gives:
  
  channel type=spicevmc
target type=channel-target-virtio/
  /channel
  
  Where we expect this:
  
  channel type='spicevmc'
target type='virtio' name='com.redhat.spice.0'/
address type='virtio-serial' controller='0' bus='0' port='1'/
  /channel
 
 The address element is optional, see
 http://libvirt.org/formatdomain.html#elementCharChannel The optional
 address element can tie the channel to a particular type='virtio-serial'
 controller.
 It seems libvirt will still do the right thing if it's omitted:
 http://libvirt.org/formatdomain.html#elementsAddress
 I tend to only add API in libvirt-gconfig when there's a need for it, but I
 can look into adding API to set the address element if you think that's
 needed now.

Yes, there is no need to specify an address element really. Just
let libvirt do its thing.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Christophe Fergeau
On Tue, Mar 13, 2012 at 04:31:56PM +0100, Christophe Fergeau wrote:
 On Tue, Mar 13, 2012 at 04:18:21PM +0100, Marc-André Lureau wrote:
  On Tue, Mar 13, 2012 at 4:15 PM, Christophe Fergeau cferg...@redhat.com 
  wrote:
         target type=channel-target-virtio/
  
  That's what I was mainly looking at, and I wish the test would cover a
  more complex and needed case, just to be sure.
 
 Ah, this is fixed by the first patch in the series, I hadn't noticed it.

I've now pushed this first patch to master since it had been ack'ed by
Zeeshan already.

Christophe


pgp0Ggzqz3hi0.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Christophe Fergeau
On Tue, Mar 13, 2012 at 04:18:21PM +0100, Marc-André Lureau wrote:
 On Tue, Mar 13, 2012 at 4:15 PM, Christophe Fergeau cferg...@redhat.com 
 wrote:
        target type=channel-target-virtio/
 
 That's what I was mainly looking at, and I wish the test would cover a
 more complex and needed case, just to be sure.

Ah, this is fixed by the first patch in the series, I hadn't noticed it.

Christophe


pgpXu9cJg62FF.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Do not enforce source type of console[0]

2012-03-13 Thread Jan Kiszka
If console[0] is an alias for serial[0], do not enforce the former to
have a PTY source type. This breaks serial consoles on stdio and makes
no sense.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 src/qemu/qemu_process.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ef311d1..216b594 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1376,7 +1376,6 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm,
 if ((ret = virDomainChrSourceDefCopy(chr-source,
  
((vm-def-serials[0])-source))) != 0)
 return ret;
-chr-source.type = VIR_DOMAIN_CHR_TYPE_PTY;
 } else {
 if (chr-source.type == VIR_DOMAIN_CHR_TYPE_PTY 
 chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
-- 
1.7.3.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Marc-André Lureau
So it looks ok to me, but

On Tue, Mar 13, 2012 at 4:15 PM, Christophe Fergeau cferg...@redhat.com wrote:
 I tend to only add API in libvirt-gconfig when there's a need for it, but I
 can look into adding API to set the address element if you think that's
 needed now.

How do you verify new_from_xml()? Am I missing something?

-- 
Marc-André Lureau

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Christophe Fergeau
On Tue, Mar 13, 2012 at 05:01:14PM +0100, Marc-André Lureau wrote:
 How do you verify new_from_xml()? Am I missing something?

This is just copy and paste, and keeping all files consistent. I don't
think we have any user of these methods, except the top level ones, and I'm
not sure it's really useful to be able to generate config objects from xml
fragments. So basically it's boilerplate for now and not really useful.
Maybe I should kill all these funcs some day...


Christophe


pgpnEoaEzXSAy.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib 2/3] Add LibvirtGConfigDomainChardevSourceSpiceVmc

2012-03-13 Thread Daniel P. Berrange
On Tue, Mar 13, 2012 at 06:37:04PM +0100, Christophe Fergeau wrote:
 On Tue, Mar 13, 2012 at 05:01:14PM +0100, Marc-André Lureau wrote:
  How do you verify new_from_xml()? Am I missing something?
 
 This is just copy and paste, and keeping all files consistent. I don't
 think we have any user of these methods, except the top level ones, and I'm
 not sure it's really useful to be able to generate config objects from xml
 fragments. So basically it's boilerplate for now and not really useful.
 Maybe I should kill all these funcs some day...

It might be useful when you get into the realm of hotplug, since the
virDomain{Attach,Update,Detach}Device APIs use XML fragments for
just the invididual devices.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] apparmor: QEMU bridge helper policy updates

2012-03-13 Thread Jamie Strandboge
On Tue, 2012-03-13 at 08:42 -0400, Corey Bryant wrote:
 This patch provides AppArmor policy updates for the QEMU bridge helper.
 The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
 capabilities to CAP_NET_ADMIN and adds a tap device to a network
 bridge. For more details on the helper, please refer to:
 
 http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03562.html
 
 Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
 ---
  examples/apparmor/libvirt-qemu |   22 +-
  1 files changed, 21 insertions(+), 1 deletions(-)
 
 diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
 index 10cdd36..c5a11b6 100644
 --- a/examples/apparmor/libvirt-qemu
 +++ b/examples/apparmor/libvirt-qemu
 @@ -1,4 +1,4 @@
 -# Last Modified: Mon Apr  5 15:11:27 2010
 +# Last Modified: Fri Mar  9 14:43:22 2012
  
#include abstractions/base
#include abstractions/consoles
 @@ -108,3 +108,23 @@
/bin/dash rmix,
/bin/dd rmix,
/bin/cat rmix,
 +
 +  /usr/libexec/qemu-bridge-helper Cx,
 +
 +  # child profile for bridge helper process
 +  profile /usr/libexec/qemu-bridge-helper {
 +#include abstractions/base
 +
 +capability setuid,
 +capability setgid,
 +capability setpcap,
 +capability net_admin,
 +
 +network inet stream,
 +
 +/dev/net/tun rw,
 +/etc/qemu/** r,
 +owner @{PROC}/*/status r,
 +
 +/usr/libexec/qemu-bridge-helper rmix,
 +  }

The policy looks good to me. Thanks! It might make more sense to have
this committed when libvirt has qemu-bridge-helper, but others can
decide on that.

Acked-By: Jamie Strandboge ja...@canonical.com


-- 
Jamie Strandboge | http://www.canonical.com


signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] cpu: Add cpu definition for Intel Sandy Bridge cpu type

2012-03-13 Thread Eric Blake
On 03/07/2012 06:30 AM, Peter Krempa wrote:
 This patch adds support for the new tsc-deadline feature flag
 and a new model to the supported model list describing the
 Intel Sandy Bridge platform.
 ---

ACK.  That promised followup patch that sorts things into bit order
would be helpful :)

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/9] qemu: Do not start with source for removable disks if tray is open

2012-03-13 Thread Eric Blake
On 03/11/2012 08:44 AM, Paolo Bonzini wrote:
 Il 05/03/2012 11:25, Osier Yang ha scritto:
 This is similiar with physical world, one will be surprised if the
 box starts with medium exists while the tray is open.

 New tests are added, tests disk-{cdrom,floppy}-tray are for the qemu
 supports -device flag, and disk-{cdrom,floppy}-no-device-cap are
 for old qemu, i.e. which doesn't support -device flag.
 
 If the disk type is block, and the source drive is a CD-ROM, the
 virtual tray state should be tied to the physical tray, even though this
 isn't always the case due to QEMU bugs.
 
 Perhaps you should fail creation if the tray attribute is open in the
 above circumstances.  Another possibility is to forbid specifying the
 tray attribute completely.
 
 Or you can just drop this patch, and only print the tray state in the
 virsh dumpxml output.  There are other attributes that already handled
 this way.

Are we trying to map the tray='open' to what the guest sees (in which
case, we should reject it for non-cdrom guest views), what the host sees
(even if the guest is viewing the storage as a non-cdrom IDE disk, but
the host storage backing that disk is a cdrom), or both?

I would argue that target tray='open'/ should describe _only_ the
guest's view, regardless of host state (if host is even tying a physical
cdrom to the guest), and that if we _need_ the host state, that it
should be an optional element in source.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] apparmor: QEMU bridge helper policy updates

2012-03-13 Thread coreyb


Quoting Jamie Strandboge ja...@canonical.com:
...


The policy looks good to me. Thanks! It might make more sense to have
this committed when libvirt has qemu-bridge-helper, but others can
decide on that.

Acked-By: Jamie Strandboge ja...@canonical.com


--
Jamie Strandboge | http://www.canonical.com


Thanks!

Regards,
Corey



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] RFC: mirrored live block migration in libvirt 0.9.11

2012-03-13 Thread Eric Blake
Here's what I'm planning on implementing for libvirt 0.9.11 to support
oVirt's desire to do live block migration, and built on top of qemu
1.1's new 'transaction' QMP monitor command.  Comments are welcome
before I actually post patches.

Background
==
Here is oVirt's description of mirrored live storage migration:
http://www.ovirt.org/wiki/Features/Design/StorageLiveMigration

The idea is that at all points in time, at least one storage domain has
a consistent view of all data in use by the guest.  That way, if
something fails and has to be restarted, oVirt can tell libvirt to
create a new transient domain that points to the storage domain with
consistent data, and restart the migration process, rather than the
post-copy approach that would spread data across two storage domains at
once.

For more background, here is the qemu feature page for the 'transaction'
monitor command; that wiki page includes a section which summarizes the
impacts to libvirt as proposed in this email:
http://wiki.qemu.org/Features/SnapshotsMultipleDevices

One of the goals of this proposal is to add mirrored live block
migration without adding any new API, so that the feature can be
backported to any distro that ships with the API in libvirt 0.9.10.

My proposals for libvirt 0.9.11
===
Libvirt will probe qemu to see if it knows the 'transaction' monitor
command, and set a bit in qemuCaps accordingly.

virDomainSnapshotCreateXML will learn a new flag:
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC.  If this flag is present, then
libvirt guarantees that the snapshot operation will either succeed, or
that failure will be reported without changing domain XML or qemu
runtime state.  If present, the creation API will fail if qemu lacks the
'transaction' command and more than one disk snapshot was requested in
the domainsnapshot XML.  If this flag is not present, then libvirt
will use 'transaction' if available, but fall back to
'blockdev-snapshot-sync', so that it works with older qemu, but where
the caller then has to check virDomainGetXMLDesc on failure to see if a
partial snapshot occurred.  This flag will be implied by any other part
of the API that requires the use of 'transaction'.

The VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT flag was added to
virDomainSnapshotCreateXML in 0.9.10, with semantics that it would stop
libvirt from complaining if a regular file already existed as the
snapshot destination, but without interacting with qemu, which would
blindly overwrite the contents of that file.  Since this flag is
relatively new, and has not had much use, I propose to slightly alter
its documented semantics to now interact with the qemu 1.1 feature being
added as part of 'transaction'.  If qemu supports 'transaction', then
presence of this flag implies that libvirt will explicitly request
'mode':'existing' for each snapshot, which tells qemu to open the
existing file without writing any new metadata, and that the caller is
responsible to ensure that the file has identical guest contents
(generally by creating a qcow2 file with the current file as backing
image and no additional contents).  Additionally, libvirt will now
require the file to already exist (in 0.9.10, libvirt silently ignored
the fact if the flag was requested but the file did not exist).
Presence of the flag without qemu support for 'transaction' will now
fail (that is, VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT will now imply
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC).  Absence of the flag means that
libvirt will rely on qemu's default to 'mode':'absolute-paths', and will
require that the file does not exist as a regular file; this maps to
qemu 1.0 always writing a new qcow2 header with absolute backing file
name.  If we want to later expose additional modes, like
'no-backing-file', it would be done via per-disk annotations in the
domainsnapshot XML rather than via new flags, but for this proposal, I
think oVirt is okay using the flag to set a single policy for all disks
mentioned in a given snapshot request.

virDomainSnapshotCreateXML's xml argument, domainsnapshot, will learn
an optional mirror sub-element to each disk.  While the
'transaction' command supports multiple mirrors in one transaction, for
now, libvirt will enforce at most one mirror, which should be sufficient
for oVirt's needs.  (Adding more support for the rest of the power of
'transaction' is probably best left for new libvirt API, but that's
outside the scope of this proposal).  As an example,
 domainsnapshot
   disks
 disk name='/src/base.img' snapshot='external'
   source file='/src/snap.img'/
   mirror file='/dest/snap.img'/
 /disk
   /disks
 /domainsnapshot
would create a new libvirt snapshot object with /src/snap.img as the
read-write new image, and /dest/snap.img as the new write-only mirror.
On success, this rewrites the domain's live XML to point to
/src/snap.img as its current file.

Finally, virDomainSnapshotDelete will learn a new flag,

Re: [libvirt] [Qemu-devel] Modern CPU models cannot be used with libvirt

2012-03-13 Thread Ayal Baron


- Original Message -
 On 03/12/2012 10:19 PM, Ayal Baron wrote:
 
 
  - Original Message -
  On 03/12/2012 02:12 PM, Itamar Heim wrote:
  On 03/12/2012 09:01 PM, Anthony Liguori wrote:
 
  It's a trade off. From a RAS perspective, it's helpful to have
  information about the host available in the guest.
 
  If you're already exposing a compatible family, exposing the
  actual
  processor seems to be worth the extra effort.
 
  only if the entire cluster is (and will be?) identical cpu.
 
  At least in my experience, this isn't unusual.
 
  I can definitely see places choosing homogeneous hardware and
  upgrading every few years.
  Giving them max capabilities for their cluster sounds logical to
  me.
  Esp. cloud providers.
 
 they would get same performance as from the matching cpu family.
 only difference would be if the guest known the name of the host cpu.
 
 
 
  or if you don't care about live migration i guess, which could be
  hte case for
  clouds, then again, not sure a cloud provider would want to
  expose
  the physical
  cpu to the tenant.
 
  Depends on the type of cloud you're building, I guess.
 
 
  Wouldn't this affect a simple startup of a VM with a different CPU
  (if motherboard changed as well cause reactivation issues in
  windows and fun things like that)?
 
 that's an interesting question, I have to assume this works though,
 since we didn't see issues with changing the cpu family for guests so
 far.
 

assumption... :)
I'd try changing twice in a row (run VM, stop, change family, restart VM, stop, 
change family restart VM).

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Modified version of the libvirt-test-api wrapper

2012-03-13 Thread Lucas Meneghel Rodrigues

Hi Guannan:

I've worked on your first version of the libvirt-test-api wrapper for 
autotest. Could you please check if you like the modified version?


https://github.com/autotest/autotest/pull/230

If you do think it's fine, you can ack it, or you might take it, modify 
and resend it. On a git branch, you can do the following:


curl https://github.com/autotest/autotest/pull/230.patch | git am

And then modify and resend.

Cheers,

Lucas

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH][TCK] add tests for network interface transaction

2012-03-13 Thread Xiaoqiang Hu
add tests for network interface transaction: interface_change_begin,
interface_change_commit and interface_change_rollback

---
 .../networks/110-interface-change-transaction.t|   81 
 1 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 scripts/networks/110-interface-change-transaction.t

diff --git a/scripts/networks/110-interface-change-transaction.t 
b/scripts/networks/110-interface-change-transaction.t
new file mode 100644
index 000..f518d9f
--- /dev/null
+++ b/scripts/networks/110-interface-change-transaction.t
@@ -0,0 +1,81 @@
+# -*- perl -*-
+#
+# Copyright (C) 2012-2013 Red Hat, Inc.
+# Copyright (C) 2012-2013 Xiaoqiang Hu x...@redhat.com
+#
+# This program is free software; You can redistribute it and/or modify
+# it under the GNU General Public License as published by the Free
+# Software Foundation; either version 2, or (at your option) any
+# later version
+#
+# The file LICENSE distributed along with this file provides full
+# details of the terms and conditions
+#
+
+=pod
+
+=head1 NAME
+
+networks/110-interface-lifecycle.t: test transaction for changing the
+configuration of one or more network interfaces
+
+=head1 DESCRIPTION
+
+The test case validates the transaction for changing the configuration
+of one or more network interfaces
+
+=cut
+
+use strict;
+use warnings;
+
+use Test::More tests = 2;
+
+use Sys::Virt::TCK;
+use Test::Exception;
+
+my $network_script_dir = /etc/sysconfig/network-scripts;
+my $test_interface_name = ifcfg-interface-tck-test;
+my $test_interface_cfg = $network_script_dir./.$test_interface_name;
+my $tck = Sys::Virt::TCK-new();
+my $conn = eval { $tck-setup(); };
+BAIL_OUT failed to setup test harness: $@ if $@;
+END {
+$tck-cleanup if $tck;
+unlink $test_interface_cfg if -f $test_interface_cfg;
+}
+
+my $ret;
+
+unlink $test_interface_cfg if -f $test_interface_cfg;
+
+eval { $conn-interface_change_begin(); };
+SKIP: {
+skip interface_change_begin/commit/rollback not implemented, 2 if $@  
err_not_implemented($@);
+
+$ret = system(cat EOF  $test_interface_cfg
+DEVICE=\interface-tck-test\
+BOOTPROTO=\none\
+ONBOOT=\no\
+EOF
+);
+
+$conn-interface_change_rollback();
+ok(! -e $test_interface_cfg, interface rollback);
+
+unlink $test_interface_cfg if -f $test_interface_cfg;
+
+$conn-interface_change_begin();
+
+$ret = system(cat EOF  $test_interface_cfg
+DEVICE=\interface-tck-test\
+BOOTPROTO=\none\
+ONBOOT=\no\
+EOF
+);
+
+$conn-interface_change_commit();
+ok(-e $test_interface_cfg, interface commit);
+}
+
+# end
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Emit graphics events when a SPICE client connects/disconnects

2012-03-13 Thread Laine Stump
Wire up the domain graphics event notifications for SPICE. Adapted
from a RHEL-only patch written by Dan Berrange that used custom
__com.redhat_SPICE events - equivalent events are now available in
upstream QEMU (including a SPICE_CONNECTED event, which was missing in
the __COM.redhat_SPICE version).

* src/qemu/qemu_monitor_json.c: Wire up SPICE graphics events
---
 src/qemu/qemu_monitor_json.c |   56 +++---
 1 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1a0ee94..a5ef1d4 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -59,6 +59,9 @@ static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr 
mon, virJSONValuePtr
 static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr 
data);
+static void qemuMonitorJSONHandleSPICEConnect(qemuMonitorPtr mon, 
virJSONValuePtr data);
+static void qemuMonitorJSONHandleSPICEInitialize(qemuMonitorPtr mon, 
virJSONValuePtr data);
+static void qemuMonitorJSONHandleSPICEDisconnect(qemuMonitorPtr mon, 
virJSONValuePtr data);
 
 static struct {
 const char *type;
@@ -75,6 +78,9 @@ static struct {
 { VNC_INITIALIZED, qemuMonitorJSONHandleVNCInitialize, },
 { VNC_DISCONNECTED, qemuMonitorJSONHandleVNCDisconnect, },
 { BLOCK_JOB_COMPLETED, qemuMonitorJSONHandleBlockJob, },
+{ SPICE_CONNECTED, qemuMonitorJSONHandleSPICEConnect, },
+{ SPICE_INITIALIZED, qemuMonitorJSONHandleSPICEInitialize, },
+{ SPICE_DISCONNECTED, qemuMonitorJSONHandleSPICEDisconnect, },
 };
 
 
@@ -624,7 +630,7 @@ VIR_ENUM_IMPL(qemuMonitorGraphicsAddressFamily,
   VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_LAST,
   ipv4, ipv6, unix);
 
-static void qemuMonitorJSONHandleVNC(qemuMonitorPtr mon, virJSONValuePtr data, 
int phase)
+static void qemuMonitorJSONHandleGraphics(qemuMonitorPtr mon, virJSONValuePtr 
data, int phase)
 {
 const char *localNode, *localService, *localFamily;
 const char *remoteNode, *remoteService, *remoteFamily;
@@ -643,14 +649,38 @@ static void qemuMonitorJSONHandleVNC(qemuMonitorPtr mon, 
virJSONValuePtr data, i
 }
 
 authScheme = virJSONValueObjectGetString(server, auth);
+if (!authScheme) {
+VIR_WARN(missing auth scheme in graphics event);
+return;
+}
 
 localFamily = virJSONValueObjectGetString(server, family);
+if (!authScheme) {
+VIR_WARN(missing local address family in graphics event);
+return;
+}
 localNode = virJSONValueObjectGetString(server, host);
+if (!authScheme) {
+VIR_WARN(missing local hostname in graphics event);
+return;
+}
 localService = virJSONValueObjectGetString(server, service);
+if (!localService)
+localService = ; /* Spice has multiple ports, so this isn't provided 
*/
 
 remoteFamily = virJSONValueObjectGetString(client, family);
+if (!authScheme) {
+VIR_WARN(missing remote address family in graphics event);
+return;
+}
 remoteNode = virJSONValueObjectGetString(client, host);
+if (!authScheme) {
+VIR_WARN(missing remote hostname in graphics event);
+return;
+}
 remoteService = virJSONValueObjectGetString(client, service);
+if (!remoteService)
+remoteService = ; /* Spice has multiple ports, so this isn't 
provided */
 
 saslUsername = virJSONValueObjectGetString(client, sasl_username);
 x509dname = virJSONValueObjectGetString(client, x509_dname);
@@ -672,19 +702,37 @@ static void qemuMonitorJSONHandleVNC(qemuMonitorPtr mon, 
virJSONValuePtr data, i
 
 static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr mon, 
virJSONValuePtr data)
 {
-qemuMonitorJSONHandleVNC(mon, data, VIR_DOMAIN_EVENT_GRAPHICS_CONNECT);
+qemuMonitorJSONHandleGraphics(mon, data, 
VIR_DOMAIN_EVENT_GRAPHICS_CONNECT);
 }
 
 
 static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, 
virJSONValuePtr data)
 {
-qemuMonitorJSONHandleVNC(mon, data, VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE);
+qemuMonitorJSONHandleGraphics(mon, data, 
VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE);
 }
 
 
 static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, 
virJSONValuePtr data)
 {
-qemuMonitorJSONHandleVNC(mon, data, VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT);
+qemuMonitorJSONHandleGraphics(mon, data, 
VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT);
+}
+
+
+static void qemuMonitorJSONHandleSPICEConnect(qemuMonitorPtr mon, 
virJSONValuePtr data)
+{
+qemuMonitorJSONHandleGraphics(mon, data, 
VIR_DOMAIN_EVENT_GRAPHICS_CONNECT);
+}
+
+
+static void qemuMonitorJSONHandleSPICEInitialize(qemuMonitorPtr mon, 
virJSONValuePtr data)
+{
+qemuMonitorJSONHandleGraphics(mon, data, 
VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE);
+}