Re: [libvirt] [PATCH] qemu: Fix (managed)save and snapshots with host mode CPU
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
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
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
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
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.
-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.
-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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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]
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
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
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
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
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
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
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
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
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
- 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
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
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
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); +}