[libvirt] [libvirt-php][PATCH v1] fix installation prefix
when prefix present all stuff installed to /usr/usr/ Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru --- src/Makefile.am | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index cab0456..8e6a800 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -6,10 +6,10 @@ WL=@WL@ SHLIB_FLAGS=@SHLIB_FLAGS@ install-exec-local: - $(MKDIR_P) $(DESTDIR)$(prefix)$(PHPEDIR) - $(MKDIR_P) $(DESTDIR)$(prefix)$(PHPCDIR) - $(INSTALL) -m 644 -D .libs/$(PACKAGE).so $(DESTDIR)$(prefix)$(PHPEDIR)/$(PACKAGE).so - $(INSTALL) -m 755 -d $(DESTDIR)$(prefix)$(PHPCDIR) + $(MKDIR_P) $(DESTDIR)$(PHPEDIR) + $(MKDIR_P) $(DESTDIR)$(PHPCDIR) + $(INSTALL) -m 644 -D .libs/$(PACKAGE).so $(DESTDIR)$(PHPEDIR)/$(PACKAGE).so + $(INSTALL) -m 755 -d $(DESTDIR)$(PHPCDIR) $(ECHO) extension=$(PACKAGE).so libvirt-php.ini $(ECHO) libvirt-php.ini $(ECHO) [libvirt] libvirt-php.ini @@ -19,11 +19,11 @@ install-exec-local: $(ECHO) libvirt.image_path=/var/lib/libvirt/images libvirt-php.ini $(ECHO) ; Limit maximum number of libvirt connections libvirt-php.ini $(ECHO) libvirt.max_connections=5 libvirt-php.ini - $(INSTALL) -m 644 -D libvirt-php.ini $(DESTDIR)$(prefix)$(PHPCDIR)/$(PACKAGE).ini + $(INSTALL) -m 644 -D libvirt-php.ini $(DESTDIR)$(PHPCDIR)/$(PACKAGE).ini uninstall-local: - $(RM) -f $(DESTDIR)$(prefix)$(PHPCDIR)/$(PACKAGE).ini - $(RM) -f $(DESTDIR)$(prefix)$(PHPEDIR)/$(PACKAGE).so + $(RM) -f $(DESTDIR)$(PHPCDIR)/$(PACKAGE).ini + $(RM) -f $(DESTDIR)$(PHPEDIR)/$(PACKAGE).so AM_CFLAGS = \ $(PHPINC) $(LIBXML_CFLAGS) \ -- 2.3.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] docs: Properly mark acl.html dependencies
On Tue, Jun 23, 2015 at 14:46:36 +0200, Martin Kletzander wrote: On Tue, Jun 23, 2015 at 01:56:00PM +0200, Michal Privoznik wrote: The acl.html file includes aclperms.htmlinc which is generated. However, acl.html is generated too from acl.html.tmp. And in fact, this is the place where the aclperms file is needed. Fix the dependency in Makefile. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- ACK (I thought I ACKed the previous one with this modification, I don't know why :). diff to v1: - Fix the origin of the error docs/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index b7b49cb..13dddf8 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -163,7 +163,7 @@ EXTRA_DIST= \ sitemap.html.in aclperms.htmlinc \ todo.pl hvsupport.pl todo.cfg-example -acl.html:: $(srcdir)/aclperms.htmlinc +acl.html.tmp: $(srcdir)/aclperms.htmlinc Now after a build I get: $ git status On branch master Your branch is up-to-date with 'origin/master'. Untracked files: (use git add file... to include in what will be committed) docs/acl.html.tmp nothing added to commit but untracked files present (use git add to track) signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: blockcopy: Forbid using persistent bitmap granularity with raw vols
qemu returns error but only in the event after the block job actually starts. Reject it upfront for a better error message. Instead of: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: Block Copy unexpectedly failed You will now get: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: unsupported configuration: granularity can't be used with target volume format Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1233115 --- src/qemu/qemu_driver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c1373de..f570879 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16671,6 +16671,16 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, goto endjob; } +/* blacklist granularity with some known-bad formats */ +if (granularity +(mirror-format == VIR_STORAGE_FILE_RAW || + (mirror-format = VIR_STORAGE_FILE_NONE + disk-src-format == VIR_STORAGE_FILE_RAW))) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(granularity can't be used with target volume format)); +goto endjob; +} + /* Prepare the destination file. */ /* XXX Allow non-file mirror destinations */ if (!virStorageSourceIsLocalStorage(mirror)) { -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virt-aa-helper: fix rules for paths with trailing slash
Rules generated for a path like '/' were having '//' which isn't correct for apparmor. Make virt-aa-helper smarter to avoid these. --- src/security/virt-aa-helper.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..9f1c570 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -773,6 +773,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi char *tmp = NULL; int rc = -1; bool readonly = true; +bool trailingSlash; if (path == NULL) return rc; @@ -809,14 +810,18 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi goto cleanup; } -virBufferAsprintf(buf, \%s%s\ %s,\n, tmp, recursive ? /** : , perms); +trailingSlash = (tmp[strlen(tmp) - 1] == '/'); + +virBufferAsprintf(buf, \%s%s%s\ %s,\n, tmp, trailingSlash ? : /, + recursive ? ** : , perms); if (readonly) { virBufferAddLit(buf, # don't audit writes to readonly files\n); -virBufferAsprintf(buf, deny \%s%s\ w,\n, tmp, recursive ? /** : ); +virBufferAsprintf(buf, deny \%s%s%s\ w,\n, tmp, + trailingSlash ? : /, recursive ? ** : ); } if (recursive) { /* allow reading (but not creating) the dir */ -virBufferAsprintf(buf, \%s/\ r,\n, tmp); +virBufferAsprintf(buf, \%s%s\ r,\n, tmp, trailingSlash ? : /); } cleanup: -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:lxc:xml:libxl:xen: improve the error in openconsole/channel
On Mon, Jun 15, 2015 at 09:58:36PM +0800, Luyao Huang wrote: We allow do not pass the dev_name to openconsole() and openchannel() function, but the error message is not good when we do not specified the console/channel name. the error message after this patch: error: internal error: character device serial0 is not using a PTY Signed-off-by: Luyao Huang lhu...@redhat.com --- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_driver.c | 3 ++- src/qemu/qemu_driver.c | 8 src/uml/uml_driver.c | 3 ++- src/xen/xen_driver.c | 3 ++- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a7be745..c64d9be 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4292,14 +4292,14 @@ libxlDomainOpenConsole(virDomainPtr dom, if (!chr) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot find character device %s), - NULLSTR(dev_name)); + dev_name ? dev_name : to open); goto cleanup; } NACK to this hunk as that would be untranslatable. And not just because to open would stay as-is. if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_INTERNAL_ERROR, _(character device %s is not using a PTY), - NULLSTR(dev_name)); + dev_name ? dev_name : NULLSTR(chr-info.alias)); goto cleanup; } The rest look pretty fine, though, so I'll push that part in a while. With fixed-up commit message... Martin signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu:conf: introduce a function to delete vcpu sched
https://bugzilla.redhat.com/show_bug.cgi?id=1235180 We have API allow vpu to be deleted, but an vcpu may be included in some domain vcpu sched, so add a new API to allow removing an iothread from some entry. Split the virDomainIOThreadSchedDelId to reuse some code. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 48 ++-- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 6 -- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 183e66c..7a464a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17996,29 +17996,49 @@ virDomainIOThreadIDDel(virDomainDefPtr def, } } -void -virDomainIOThreadSchedDelId(virDomainDefPtr def, -unsigned int iothreadid) +static void +virDomainThreadSchedDelId(virDomainThreadSchedParamPtr *threadsched, + size_t *nthreadsched, + unsigned int id) { size_t i; -if (!def-cputune.iothreadsched || !def-cputune.niothreadsched) -return; - -for (i = 0; i def-cputune.niothreadsched; i++) { -if (virBitmapIsBitSet(def-cputune.iothreadsched[i].ids, iothreadid)) { -ignore_value(virBitmapClearBit(def-cputune.iothreadsched[i].ids, - iothreadid)); -if (virBitmapIsAllClear(def-cputune.iothreadsched[i].ids)) { -virBitmapFree(def-cputune.iothreadsched[i].ids); -VIR_DELETE_ELEMENT(def-cputune.iothreadsched, i, - def-cputune.niothreadsched); +for (i = 0; i *nthreadsched; i++) { +if (virBitmapIsBitSet((*threadsched)[i].ids, id)) { +ignore_value(virBitmapClearBit((*threadsched)[i].ids, id)); +if (virBitmapIsAllClear((*threadsched)[i].ids)) { +virBitmapFree((*threadsched)[i].ids); +VIR_DELETE_ELEMENT((*threadsched), i, *nthreadsched); } return; } } } +void +virDomainIOThreadSchedDelId(virDomainDefPtr def, +unsigned int iothreadid) +{ +if (!def-cputune.iothreadsched || !def-cputune.niothreadsched) +return; + +virDomainThreadSchedDelId(def-cputune.iothreadsched, + def-cputune.niothreadsched, + iothreadid); +} + +void +virDomainVcpuSchedDelId(virDomainDefPtr def, +unsigned int vcpuid) +{ +if (!def-cputune.vcpusched || !def-cputune.nvcpusched) +return; + +virDomainThreadSchedDelId(def-cputune.vcpusched, + def-cputune.nvcpusched, + vcpuid); +} + virDomainPinDefPtr virDomainPinFind(virDomainPinDefPtr *def, int npin, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c96a6e4..a74dbc9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2662,6 +2662,7 @@ virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id); void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id); void virDomainIOThreadSchedDelId(virDomainDefPtr def, unsigned int iothread_id); +void virDomainVcpuSchedDelId(virDomainDefPtr def, unsigned int vcpuid); unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1566d11..169d641 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -479,6 +479,7 @@ virDomainTPMBackendTypeToString; virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; +virDomainVcpuSchedDelId; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c1373de..245443c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4966,12 +4966,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (persistentDef) { -/* remove vcpupin entries for vcpus that were unplugged */ +/* remove vcpupin and vcpusched entries for vcpus that were unplugged */ if (nvcpus persistentDef-vcpus) { -for (i = persistentDef-vcpus - 1; i = nvcpus; i--) +for (i = persistentDef-vcpus - 1; i = nvcpus; i--) { virDomainPinDel(persistentDef-cputune.vcpupin, persistentDef-cputune.nvcpupin, i); +virDomainVcpuSchedDelId(persistentDef, i); +} } if (flags VIR_DOMAIN_VCPU_MAXIMUM) { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com
Re: [libvirt] [PATCH 01/13] qemu: refactor qemuBuildControllerDevStr to eliminate future duplicate code
On Mon, Jun 22, 2015 at 02:44:06PM -0400, Laine Stump wrote: The PCI case of the switch statement in this function contains another switch statement with a case for each model. Currently every model except pci-root and pcie-root have a check for index 0 (since only every model has those two can have index==0), and the function should never be called for those two anyway. If we move the check for !pci[e]-root to the top of the pci case, then we can move the check for index 0 out of the individual model cases. This will save repeating that check for the three new controller models about to be added. --- src/qemu/qemu_command.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: Fix trivial copy-paste error
Signed-off-by: Martin Kletzander mklet...@redhat.com --- Pushed as trivial (in a while). docs/formatdomain.html.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 95d8c45487aa..f7b9f519a377 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -726,8 +726,7 @@ If no codeiothreadids/code are defined, then libvirt numbers IOThreads from 1 to the number of codeiothreads/code available for the domain. For real-time schedulers (codefifo/code, -coderr/code), priority must real-time schedulers -(codefifo/code, coderr/code), priority must be specified as +coderr/code), priority must be specified as well (and is ignored for non-real-time ones). The value range for the priority depends on the host kernel (usually 1-99). span class=sinceSince 1.2.13/span -- 2.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
On Tue, Jun 23, 2015 at 09:06:24PM -0400, Laine Stump wrote: On 06/23/2015 11:57 AM, Ján Tomko wrote: On Mon, Jun 22, 2015 at 02:44:05PM -0400, Laine Stump wrote: [snip] === Idea 3: interpret controllers with missing subType as above, but actually write it out to the config/migration/etc in the new modified format. Cons: This would prevent downgrading libvirt or migrating from a host with newer libvirt to one with older libvirt. (Although preserving compatibility at some level when downgrading may be a stated requirement of some downstream distros' builds of libvirt, I think for upstream it is only a best effort; I'm just not certain how much best is considered to be :-) I do not know of any effort done to make downgrading libvirt work. You haven't talked to enough people deploying RHEV/oVirt in production - they want the ability to upgrade some nodes, migrate guests over to them, then migrate the guests back if the upgrade needs to be undone. That is migration, where we pass the migratable XML. The domain configs and live status XMLs can contain things that won't be parsable by older libvirt. Under 'downgrade' I imagined simply installing older libvirt packages - this can possibly fail, even if the domains only use features present in the old libvirt too. Any machine configs that use new values for old attributes will disappear (and so will running machines, because of new qemu capabilities). Migration is somewhat supported, so the compatible format could be used only when the model is default and VIR_DOMAIN_DEF_FORMAT_MIGRATABLE was specified? Isn't VIR_DOMAIN_DEF_FORMAT_MIGRATABLE there in part so that migrations from newer libvirt to older libvirt might work? (it doesn't guarantee it, but it does make it work in some situations where it otherwise wouldn't). Yes, if the domain worked with the old libvirt, we should be able to migrate to the new libvirt and back, thanks to this flag. === Idea 4: Unlike other uses of model in libvirt, for pci controllers, continue to use model to denote the subtype/class/whatever of controller, and create a new attribute to denote the different specific implementations of each one. So for example: [4] controller type='pci' model='dmi-to-pci-bridge'/ would become: [5] controller type='pci' model='dmi-to-pci-bridge' implementation='i82801b11-bridge'/ (or some other name in place of implementation - ideas? I'm horrible at thinkin up names) device? actualModel? :) hey, I think I might like device! Pros: wouldn't create compatibility problems when downgrading or migrating cross version. If you tried to migrate to older libvirt with: model='dmi-to-pci-bridge' impl='generic', older libvirt would not parse the impl flag and create a machine with i8201b11-bridge. (Assuming the QEMU would know the machine type). Well, yeah, this does point out a flaw in my thinking :-/ This happens with all new XML elements - libvirt's parser usually does an XPath query for the elements it knows and ignores the unknown ones. IIRC this is on purpose. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockcopy: Forbid using persistent bitmap granularity with raw vols
On 06/24/2015 03:11 PM, Peter Krempa wrote: qemu returns error but only in the event after the block job actually starts. Reject it upfront for a better error message. Instead of: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: Block Copy unexpectedly failed You will now get: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: unsupported configuration: granularity can't be used with target volume format This can't avoid the cases that the source file has a raw format file as backing file. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1233115 --- src/qemu/qemu_driver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c1373de..f570879 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16671,6 +16671,16 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, goto endjob; } +/* blacklist granularity with some known-bad formats */ +if (granularity +(mirror-format == VIR_STORAGE_FILE_RAW || + (mirror-format = VIR_STORAGE_FILE_NONE + disk-src-format == VIR_STORAGE_FILE_RAW))) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(granularity can't be used with target volume format)); +goto endjob; +} + /* Prepare the destination file. */ /* XXX Allow non-file mirror destinations */ if (!virStorageSourceIsLocalStorage(mirror)) { -- Regards shyu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:lxc:xml:libxl:xen: improve the error in openconsole/channel
On 06/24/2015 04:12 PM, Martin Kletzander wrote: On Mon, Jun 15, 2015 at 09:58:36PM +0800, Luyao Huang wrote: We allow do not pass the dev_name to openconsole() and openchannel() function, but the error message is not good when we do not specified the console/channel name. the error message after this patch: error: internal error: character device serial0 is not using a PTY Signed-off-by: Luyao Huang lhu...@redhat.com --- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_driver.c | 3 ++- src/qemu/qemu_driver.c | 8 src/uml/uml_driver.c | 3 ++- src/xen/xen_driver.c | 3 ++- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a7be745..c64d9be 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4292,14 +4292,14 @@ libxlDomainOpenConsole(virDomainPtr dom, if (!chr) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot find character device %s), - NULLSTR(dev_name)); + dev_name ? dev_name : to open); goto cleanup; } NACK to this hunk as that would be untranslatable. And not just because to open would stay as-is. Got it, thanks for pointing out that. if (chr-source.type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_INTERNAL_ERROR, _(character device %s is not using a PTY), - NULLSTR(dev_name)); + dev_name ? dev_name : NULLSTR(chr-info.alias)); goto cleanup; } The rest look pretty fine, though, so I'll push that part in a while. With fixed-up commit message... Thanks a lot for your review and help ! Martin Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/13] qemu: always permit PCI devices to be manually assigned to a PCIe bus
On Mon, Jun 22, 2015 at 02:44:07PM -0400, Laine Stump wrote: When support for the pcie-root and dmi-to-pci-bridge buses on a Q35 machinetype was added, I was concerned that even though qemu at the time allowed plugging a PCI device into a PCIe port, that it might not be supported in the future. To prevent painful backtracking in the possible future where this happened, I disallowed such connections except in a few specific cases requested by qemu developers (indicated in the code with the flag VIR_PCI_CONNEcT_TYPE_EITHER_IF_CONFIG). s/CONNEcT/CONNECT/g Now that a couple years have passed, there is a clear message from qemu that there is no danger in allowing PCI devices to be plugged into PCIe ports. This patch eliminates VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG and changes the code to always allow PCI-PCIe or PCIe-PCI connection *when the PCI address is specified in the config. (For newly added devices that haven't yet been given a PCI address, the auto-placement still prefers using the correct type of bus). --- docs/formatdomain.html.in | 18 +++--- src/conf/domain_addr.c| 18 +++--- src/conf/domain_addr.h| 16 ++-- src/qemu/qemu_command.c | 6 ++ 4 files changed, 34 insertions(+), 24 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockcopy: Forbid using persistent bitmap granularity with raw vols
On Wed, Jun 24, 2015 at 16:48:16 +0800, Shanzhi Yu wrote: On 06/24/2015 03:11 PM, Peter Krempa wrote: qemu returns error but only in the event after the block job actually starts. Reject it upfront for a better error message. Instead of: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: Block Copy unexpectedly failed You will now get: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: unsupported configuration: granularity can't be used with target volume format This can't avoid the cases that the source file has a raw format file as backing file. AFAIK if the active image (top layer) is a qcow2, then the destination is also created as a qcow2 and thus the persistent bitmap granularity should work just fine. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: improve the way we format blkiotune and cputune
On Tue, Jun 23, 2015 at 09:24:25PM +0800, Luyao Huang wrote: Just refactor existing code to use a child buf instead of check all element before format blkiotune and cputune. This will avoid the more and more bigger element check during we introduce new elements in blkiotune and cputune in the future. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 168 +++-- 1 file changed, 65 insertions(+), 103 deletions(-) Nice cleanup, ACK Pushed. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [[libvirt-php][PATCH v2] qemu-agent-command] fixes for installation and add another libvirt function
2015-06-22 15:04 GMT+03:00 Michal Privoznik mpriv...@redhat.com: These are semantically two separate changes and should go into separate patches. Can you please rebase, split and resend? Yes, thanks. I'm send two patches now. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-php][PATCH v1] add libvirt_domain_qemu_agent_command
add another missing libvirt command Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru --- configure.ac | 3 +++ src/Makefile.am | 5 +++-- src/libvirt-php.c | 28 src/libvirt-php.h | 2 ++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 9e9fee0..0afa2e2 100644 --- a/configure.ac +++ b/configure.ac @@ -23,6 +23,9 @@ PKG_PROG_PKG_CONFIG LIBVIRT_REQUIRED=0.6.2 PKG_CHECK_MODULES(LIBVIRT, libvirt = $LIBVIRT_REQUIRED) +PKG_CHECK_MODULES(QEMU, libvirt-qemu) +AC_SUBST([QEMU_CFLAGS]) +AC_SUBST([QEMU_LIBS]) dnl == dnl required minimum version of libxml2 diff --git a/src/Makefile.am b/src/Makefile.am index 8e6a800..f270ec2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -27,13 +27,14 @@ uninstall-local: AM_CFLAGS = \ $(PHPINC) $(LIBXML_CFLAGS) \ - $(LIBVIRT_CFLAGS) $(DEFINES) \ + $(LIBVIRT_CFLAGS) $(QEMU_CFLAGS) $(DEFINES) \ -I$(top_srcdir)/winsrc AM_LDFLAGS = \ $(SHLIB_LDFLAGS) \ $(LIBXML_LIBS) \ - $(LIBVIRT_LIBS) + $(LIBVIRT_LIBS) \ + $(QEMU_LIBS) lib_LTLIBRARIES = libvirt-php.la diff --git a/src/libvirt-php.c b/src/libvirt-php.c index d46fbb9..f9d5557 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -161,6 +161,7 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_domain_send_keys, NULL) PHP_FE(libvirt_domain_send_pointer_event, NULL) PHP_FE(libvirt_domain_update_device, NULL) + PHP_FE(libvirt_domain_qemu_agent_command, NULL) /* Domain snapshot functions */ PHP_FE(libvirt_domain_has_current_snapshot, NULL) PHP_FE(libvirt_domain_snapshot_create, NULL) @@ -3943,6 +3944,33 @@ PHP_FUNCTION(libvirt_domain_get_name) } /* + Function name: libvirt_domain_qemu_agent_command + Since version: 0.5.2(-1) + Description:Function is used to send qemu-ga command + Arguments: @res [resource]: libvirt domain resource, e.g. from libvirt_domain_lookup_by_*() + @timeout [int]: timeout for waiting (-2 block, -1 default, 0 no wait, 0 wait specific time + @flags [int]: ?? + Returns:String on success and FALSE on error +*/ +PHP_FUNCTION(libvirt_domain_qemu_agent_command) +{ + php_libvirt_domain *domain=NULL; + zval *zdomain; + const char *cmd; + int cmd_len; + char *ret; + long timeout = -1; + long flags = 0; + + GET_DOMAIN_FROM_ARGS(rs|l|l, zdomain, cmd, cmd_len, timeout, flags); + + ret = virDomainQemuAgentCommand(domain-domain, cmd, timeout, flags); + if (ret == NULL) RETURN_FALSE; + + RETURN_STRING(ret,1); +} + +/* Function name: libvirt_domain_get_uuid_string Since version: 0.4.1(-1) Description:Function is used to get the domain's UUID in string format diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 7c9a229..1d9c6ab 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -80,6 +80,7 @@ #include libvirt/libvirt.h #include libvirt/virterror.h +#include libvirt/libvirt-qemu.h #include libxml/parser.h #include libxml/xpath.h #include fcntl.h @@ -427,6 +428,7 @@ PHP_FUNCTION(libvirt_domain_send_keys); PHP_FUNCTION(libvirt_domain_send_pointer_event); PHP_FUNCTION(libvirt_domain_get_metadata); PHP_FUNCTION(libvirt_domain_set_metadata); +PHP_FUNCTION(libvirt_domain_qemu_agent_command); /* Domain snapshot functions */ PHP_FUNCTION(libvirt_domain_has_current_snapshot); PHP_FUNCTION(libvirt_domain_snapshot_create); -- 2.3.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Wed, Jun 10, 2015 at 12:03:44PM +0200, Johannes Holmberg wrote: --- Hello, I often find myself wanting to validate a domain xml without having to save it to a file, so I've updated virt-xml-validate to support the standard practice of reading from stdin when - is given as the input file. Of course, the validation is multipass so there still needs to be a temporary file but I think it makes sense to hide this complexity in virt-xml-validate and not force it onto the user. /Johannes It looks like this could work, just a couple things... tools/virt-xml-validate.in | 53 +- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index a04fa06..8807f8d 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -17,6 +17,16 @@ set -e +TMPFILE= + +cleanup() { + if [ $TMPFILE ]; then +rm -f $TMPFILE + fi +} + +trap cleanup EXIT + case $1 in -h | --h | --he | --hel | --help) cat EOF @@ -35,7 +45,7 @@ $0 (libvirt) @VERSION@ EOF exit ;; --) shift ;; - -*) + -?*) echo $0: unrecognized option '$1' 2 exit 1 ;; esac @@ -43,18 +53,27 @@ esac XMLFILE=$1 TYPE=$2 -if [ -z $XMLFILE ]; then - echo syntax: $0 XMLFILE [TYPE] 2 - exit 1 -fi - -if [ ! -f $XMLFILE ]; then - echo $0: document $XMLFILE does not exist 2 - exit 2 +if [ $XMLFILE = - ]; then +TMPFILE=`mktemp --tmpdir virt-xml.` +cat $TMPFILE Why don't you just set XMLFILE=$TMPFILE ? That would get rid of lot of the code changes below and would be more readable. Other than that it looks good. +else + if [ -z $XMLFILE ]; then +echo syntax: $0 XMLFILE [TYPE] 2 +exit 1 + fi + + if [ ! -f $XMLFILE ]; then +echo $0: document $XMLFILE does not exist 2 +exit 2 + fi fi if [ -z $TYPE ]; then - ROOT=`xmllint --stream --debug $XMLFILE 2/dev/null | grep ^0 1 | awk '{ print $3 }'` + if [ $TMPFILE ]; then +ROOT=`cat $TMPFILE | xmllint --stream --debug - 2/dev/null | grep ^0 1 | awk '{ print $3 }'` + else +ROOT=`xmllint --stream --debug $XMLFILE 2/dev/null | grep ^0 1 | awk '{ print $3 }'` + fi case $ROOT in *domainsnapshot*) # Must come first, since *domain* is a substring TYPE=domainsnapshot @@ -99,8 +118,11 @@ if [ ! -f $SCHEMA ]; then exit 4 fi -xmllint --noout --relaxng $SCHEMA $XMLFILE - +if [ $TMPFILE ]; then + cat $TMPFILE | xmllint --noout --relaxng $SCHEMA - +else + xmllint --noout --relaxng $SCHEMA $XMLFILE +fi exit : =cut @@ -119,9 +141,10 @@ exit Validates a libvirt XML for compliance with the published schema. The first compulsory argument is the path to the XML file to be -validated. The optional second argument is the name of the schema -to validate against. If omitted, the schema name will be inferred -from the name of the root element in the XML document. +validated (or - to read the XML from standard input). The optional +second argument is the name of the schema to validate against. If +omitted, the schema name will be inferred from the name of the root +element in the XML document. Valid schema names currently include -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Adjust invalid secrettype setting during parse
On Mon, Jun 15, 2015 at 03:00:16PM -0400, John Ferlan wrote: Commit id '1feaccf0' attempted to handle an empty secrettype value; however, it made a mistake by processing the secretType as if it was the original secrettype string. The 'secretType' is actually whether 'usage' or 'uuid' was used. Thus adjust part of the change to make the same check for def-src-type != VIR_STORAGE_TYPE_VOLUME before setting auth_secret_usage from the secrettype field. Luckily the aforementioned commits misdeed would be overwritten by the call to virStorageTranslateDiskSourcePool Signed-off-by: John Ferlan jfer...@redhat.com --- Follow up to my recent change and response, see: http://www.redhat.com/archives/libvir-list/2015-June/msg00666.html ACK src/conf/domain_conf.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ca55981..c31edf5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6571,17 +6571,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur-name, BAD_CAST auth)) { if (!(authdef = virStorageAuthDefParse(node-doc, cur))) goto error; -/* Shared processing code with storage pools can leave - * this empty, but disk formatting uses it as does command - * creation - so use the secretType to attempt to fill it in. +/* Disk volume types won't have the secrettype filled in until + * after virStorageTranslateDiskSourcePool is run */ -if (!authdef-secrettype) { -const char *secrettype = -virSecretUsageTypeToString(authdef-secretType); -if (VIR_STRDUP(authdef-secrettype, secrettype) 0) -goto error; -} -if ((auth_secret_usage = +if (def-src-type != VIR_STORAGE_TYPE_VOLUME +(auth_secret_usage = virSecretUsageTypeFromString(authdef-secrettype)) 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(invalid secret type %s), -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] Support for the new watchdog model diag288
This patch provides support for the new watchdog model diag288. Signed-off-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com Reviewed-by: Daniel Hansel daniel.han...@linux.vnet.ibm.com Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com Reviewed-by: Tony Krowiak akrow...@linux.vnet.ibm.com --- docs/formatdomain.html.in | 2 ++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 3 ++- src/conf/domain_conf.h| 1 + src/qemu/qemu_command.c | 6 +++--- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a97e246..c432aeb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5717,6 +5717,8 @@ qemu-kvm -net nic,model=? /dev/null li 'i6300esb' mdash; the recommended device, emulating a PCI Intel 6300ESB /li li 'ib700' mdash; emulating an ISA iBase IB700 /li + li 'diag288' mdash; emulating an S390 DIAG288 device +span class=sinceSince 1.3.0/span/li /ul /dd dtcodeaction/code/dt diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3040797..1120003 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3261,6 +3261,7 @@ choice valuei6300esb/value valueib700/value + valuediag288/value /choice /attribute optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 369d155..03c3f74 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -498,7 +498,8 @@ VIR_ENUM_IMPL(virDomainSmbiosMode, VIR_DOMAIN_SMBIOS_LAST, VIR_ENUM_IMPL(virDomainWatchdogModel, VIR_DOMAIN_WATCHDOG_MODEL_LAST, i6300esb, - ib700) + ib700, + diag288) VIR_ENUM_IMPL(virDomainWatchdogAction, VIR_DOMAIN_WATCHDOG_ACTION_LAST, reset, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8912bbb..aeba5a5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1304,6 +1304,7 @@ struct _virDomainSoundDef { typedef enum { VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB, VIR_DOMAIN_WATCHDOG_MODEL_IB700, +VIR_DOMAIN_WATCHDOG_MODEL_DIAG288, VIR_DOMAIN_WATCHDOG_MODEL_LAST } virDomainWatchdogModel; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5444638..99755f1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2308,7 +2308,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * - VirtIO block * - VirtIO balloon * - Host device passthrough - * - Watchdog (not IB700) + * - Watchdog * - pci serial devices * * Prior to this function being invoked, qemuCollectPCIAddress() will have @@ -2543,9 +2543,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, goto error; } -/* A watchdog - skip IB700, it is not a PCI device */ +/* A watchdog - check if it is a PCI device */ if (def-watchdog -def-watchdog-model != VIR_DOMAIN_WATCHDOG_MODEL_IB700 +def-watchdog-model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB def-watchdog-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { if (virDomainPCIAddressReserveNextSlot(addrs, def-watchdog-info, flags) 0) -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] Support for a new watchdog action inject-nmi
This patch provides support for a new watchdog action inject-nmi which allows to define an inject of a non-maskable interrupt into a guest. Signed-off-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com Reviewed-by: Daniel Hansel daniel.han...@linux.vnet.ibm.com Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com Reviewed-by: Tony Krowiak akrow...@linux.vnet.ibm.com --- docs/formatdomain.html.in| 3 +++ docs/schemas/domaincommon.rng| 1 + include/libvirt/libvirt-domain.h | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_monitor_json.c | 2 +- tools/virsh-domain.c | 3 ++- 7 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 95d8c45..a97e246 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5738,6 +5738,9 @@ qemu-kvm -net nic,model=? /dev/null li'none' mdash; do nothing/li li'dump' mdash; automatically dump the guest span class=sinceSince 0.8.7/span/li + li'inject-nmi' mdash; inject a non-maskable interrupt +into the guest +span class=sinceSince 1.3.0/span/li /ul p Note 1: the 'shutdown' action requires that the guest diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3014365..3040797 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3272,6 +3272,7 @@ valuepause/value valuenone/value valuedump/value +valueinject-nmi/value /choice /attribute /optional diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7564c20..e8202cf 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2842,6 +2842,7 @@ typedef enum { VIR_DOMAIN_EVENT_WATCHDOG_POWEROFF, /* Guest is forcibly powered off */ VIR_DOMAIN_EVENT_WATCHDOG_SHUTDOWN, /* Guest is requested to gracefully shutdown */ VIR_DOMAIN_EVENT_WATCHDOG_DEBUG,/* No action, a debug message logged */ +VIR_DOMAIN_EVENT_WATCHDOG_INJECTNMI,/* Inject a non-maskable interrupt into guest */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_WATCHDOG_LAST diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 183e66c..369d155 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -506,7 +506,8 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, VIR_DOMAIN_WATCHDOG_ACTION_LAST, poweroff, pause, dump, - none) + none, + inject-nmi) VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, vga, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c96a6e4..8912bbb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1315,6 +1315,7 @@ typedef enum { VIR_DOMAIN_WATCHDOG_ACTION_PAUSE, VIR_DOMAIN_WATCHDOG_ACTION_DUMP, VIR_DOMAIN_WATCHDOG_ACTION_NONE, +VIR_DOMAIN_WATCHDOG_ACTION_INJECTNMI, VIR_DOMAIN_WATCHDOG_ACTION_LAST } virDomainWatchdogAction; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 71e8f4b..b1ee852 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -555,7 +555,7 @@ static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr d VIR_ENUM_DECL(qemuMonitorWatchdogAction) VIR_ENUM_IMPL(qemuMonitorWatchdogAction, VIR_DOMAIN_EVENT_WATCHDOG_LAST, - none, pause, reset, poweroff, shutdown, debug); + none, pause, reset, poweroff, shutdown, debug, inject-nmi); static void qemuMonitorJSONHandleWatchdog(qemuMonitorPtr mon, virJSONValuePtr data) { diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index baf4fa3..27d62e9 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11703,7 +11703,8 @@ VIR_ENUM_IMPL(vshDomainEventWatchdog, N_(reset), N_(poweroff), N_(shutdown), - N_(debug)) + N_(debug), + N_(inject-nmi)) static const char * vshDomainEventWatchdogToString(int action) -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] Support for new watchdog model diag288 and action inject-nmi
This patch set provides support for the new watchdog model diag288 including the new watchdog action inject-nmi. v2: Added tests for action and model. Boris Fiuczynski (4): Support for a new watchdog action inject-nmi Test for the new watchdog action inject-nmi Support for the new watchdog model diag288 Test for the new watchdog model diag288 docs/formatdomain.html.in | 5 + docs/schemas/domaincommon.rng | 2 ++ include/libvirt/libvirt-domain.h | 1 + src/conf/domain_conf.c | 6 +++-- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_command.c| 6 ++--- src/qemu/qemu_monitor_json.c | 2 +- .../qemuxml2argv-watchdog-diag288.args | 8 +++ .../qemuxml2argv-watchdog-diag288.xml | 22 ++ .../qemuxml2argv-watchdog-injectnmi.args | 5 + .../qemuxml2argv-watchdog-injectnmi.xml| 26 ++ tests/qemuxml2argvtest.c | 4 tools/virsh-domain.c | 3 ++- 13 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.xml -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] Test for the new watchdog model diag288
Adding a test for the new watchdog model diag288. Signed-off-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com --- .../qemuxml2argv-watchdog-diag288.args | 8 .../qemuxml2argv-watchdog-diag288.xml | 22 ++ tests/qemuxml2argvtest.c | 3 +++ 3 files changed, 33 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.args b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.args new file mode 100644 index 000..56d74e5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +s390-virtio -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-s390,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \ +-device diag288,id=watchdog0 -watchdog-action inject-nmi diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.xml b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.xml new file mode 100644 index 000..2d9f8ac --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-diag288.xml @@ -0,0 +1,22 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219100/memory + currentMemory219100/currentMemory + os +type arch='s390x' machine='s390-virtio'hvm/type + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='virtio'/ + boot order='1'/ +/disk +watchdog model='diag288' action='inject-nmi'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 137b864..bb71991 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1186,6 +1186,9 @@ mymain(void) DO_TEST(watchdog-device, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(watchdog-dump, NONE); DO_TEST(watchdog-injectnmi, NONE); +DO_TEST(watchdog-diag288, +QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, +QEMU_CAPS_DRIVE, QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390); DO_TEST(balloon-device, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(balloon-device-auto, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/4] Test for the new watchdog action inject-nmi
Adding a test for the new watchdog action inject-nmi. Signed-off-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com --- .../qemuxml2argv-watchdog-injectnmi.args | 5 + .../qemuxml2argv-watchdog-injectnmi.xml| 26 ++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 32 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.args b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.args new file mode 100644 index 000..0f60512 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel \ +none -watchdog ib700 -watchdog-action inject-nmi diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.xml b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.xml new file mode 100644 index 000..647c456 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-injectnmi.xml @@ -0,0 +1,26 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' unit='0'/ +/disk +controller type='ide' index='0'/ +watchdog model='ib700' action='inject-nmi'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index be82dd2..137b864 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1185,6 +1185,7 @@ mymain(void) DO_TEST(watchdog, NONE); DO_TEST(watchdog-device, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(watchdog-dump, NONE); +DO_TEST(watchdog-injectnmi, NONE); DO_TEST(balloon-device, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST(balloon-device-auto, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Use heads parameter for QXL driver
On Fri, Jun 12, 2015 at 11:57:31AM +0100, Frediano Ziglio wrote: Allows to specify maximum number of head to QXL driver. Signed-off-by: Frediano Ziglio fzig...@redhat.com The patch to support the max_outputs in Qemu is still not merged but I got agreement on the name of the argument. Actually can be a compatiblity problem as heads in the XML configuration was set by default to '1'. Could we avoid that by passing the value to QEMU only if it's greater than 1? --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.replies | 8 tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 8 tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 8 tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 8 tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 8 tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 8 tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 8 Why do you put it in outputs of all QEMU versions if it's not merged yet? signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/13] docs: document when pcie-root/dmi-to-pci-bridge support was added
On Mon, Jun 22, 2015 at 02:44:09PM -0400, Laine Stump wrote: Also move the mention of version numbers for the various PCI controller models up to the end of the sentence where they are first given, to avoid confusion. --- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/13] qemu: ignore assumptions about hotplug requirement when address is from config
On Mon, Jun 22, 2015 at 02:44:08PM -0400, Laine Stump wrote: Certain PCI buses don't support hotplug, and when automatically assigning PCI addresses for devices, libvirt is very concervative in conservative its assumptions about whether or not a device will need to be hotplugged/unplugged in the future. But if the user manually assigns an address, they likely are aware of any hotplug requirements of the device (or at least they should be). In short, after this patch, automatically PCI address assignment will assume that the device must be pluggedin ot a hot-pluggable slot, but plugged in to manually assignment can place the device in any bus that is compatible, regardless of whether or not it supports hotplug. If the user makes a mistake and plugs the device into a bus that doesn't support hotplug, then later tries to do a hot-unplug, qemu will give an appropriate error. (in the future we may want to add a hotpluggable attribute to all devices, with default being yes for autoassign, and no for manual assign). --- src/conf/domain_addr.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 93c6043..2be98c5 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -50,6 +50,12 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, */ if (busFlags VIR_PCI_CONNECT_TYPES_ENDPOINT) busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT; Here, it's the bus that allows plugging both PCI and PCIe devices in. +/* Also allow manual specification of bus to override + * libvirt's assumptions about whether or not hotplug + * capability will be required. + */ +if (devFlags VIR_PCI_CONNECT_HOTPLUGGABLE) +busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE; But the bus might not be hotpluggable here. We just don't care about being able to hotplug the device. How about: devFlags ^= VIR_PCI_CONNECT_HOTPLUGGABLE; ACK either way Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [RFC] virSetUIDGID: Don't leak supplementary groups
On Tue, Jun 23, 2015 at 01:48:42PM +0200, Richard Weinberger wrote: The LXC driver uses virSetUIDGID() to become UID/GID 0. It passes an empty groups list to virSetUIDGID() to get rid of all supplementary groups from the host side. But virSetUIDGID() calls setgroups() only if the supplied list is larger than 0. This leads to a container root with unrelated supplementary groups. In most cases this issue is unoticed as libvirtd runs as UID/GID 0 without any supplementary groups. Signed-off-by: Richard Weinberger rich...@nod.at --- I've marked that patch as RFC as I'm not sure if all users of virSetUIDGID() expect this behavior too. I went through the callers and I see no reason why setgroups should not be called. ACK. I also can't think of a use case in which we'd like to keep the supplemental groups. Thanks, //richard --- src/util/virutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index cddc78a..ea697a3 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1103,7 +1103,7 @@ virSetUIDGID(uid_t uid, gid_t gid, gid_t *groups ATTRIBUTE_UNUSED, } # if HAVE_SETGROUPS -if (ngroups setgroups(ngroups, groups) 0) { +if (setgroups(ngroups, groups) 0) { virReportSystemError(errno, %s, _(cannot set supplemental groups)); return -1; -- 2.4.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Use heads parameter for QXL driver
On Fri, Jun 12, 2015 at 11:57:31AM +0100, Frediano Ziglio wrote: Allows to specify maximum number of head to QXL driver. Signed-off-by: Frediano Ziglio fzig...@redhat.com The patch to support the max_outputs in Qemu is still not merged but I got agreement on the name of the argument. Actually can be a compatiblity problem as heads in the XML configuration was set by default to '1'. Could we avoid that by passing the value to QEMU only if it's greater than 1? And what about if I just want one monitor ? Specify a one value ? --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.replies | 8 tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 8 tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 8 tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 8 tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 8 tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 8 tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 8 Why do you put it in outputs of all QEMU versions if it's not merged yet? To pass the checks Frediano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: fix rules for paths with trailing slash
On Wed, Jun 24, 2015 at 09:58:45AM +0200, Cédric Bosdonnat wrote: Rules generated for a path like '/' were having '//' which isn't That applies only for those that were recursive, right? Looks good to me, ACK. correct for apparmor. Make virt-aa-helper smarter to avoid these. --- src/security/virt-aa-helper.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..9f1c570 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -773,6 +773,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi char *tmp = NULL; int rc = -1; bool readonly = true; +bool trailingSlash; if (path == NULL) return rc; @@ -809,14 +810,18 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi goto cleanup; } -virBufferAsprintf(buf, \%s%s\ %s,\n, tmp, recursive ? /** : , perms); +trailingSlash = (tmp[strlen(tmp) - 1] == '/'); + +virBufferAsprintf(buf, \%s%s%s\ %s,\n, tmp, trailingSlash ? : /, + recursive ? ** : , perms); if (readonly) { virBufferAddLit(buf, # don't audit writes to readonly files\n); -virBufferAsprintf(buf, deny \%s%s\ w,\n, tmp, recursive ? /** : ); +virBufferAsprintf(buf, deny \%s%s%s\ w,\n, tmp, + trailingSlash ? : /, recursive ? ** : ); } if (recursive) { /* allow reading (but not creating) the dir */ -virBufferAsprintf(buf, \%s/\ r,\n, tmp); +virBufferAsprintf(buf, \%s%s\ r,\n, tmp, trailingSlash ? : /); } cleanup: -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:conf: introduce a function to delete vcpu sched
On Wed, Jun 24, 2015 at 16:44:22 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1235180 We have API allow vpu to be deleted, but an vcpu may be included in some domain vcpu sched, so add a new API to allow removing an iothread from some entry. Split the virDomainIOThreadSchedDelId to reuse some code. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 48 ++-- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 6 -- 4 files changed, 40 insertions(+), 16 deletions(-) I'm going to refactor the data structures that are storing the thread scheduler data which will inherently fix this issue so even if we apply this patch it will be basically reverted very soon. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockcopy: Forbid using persistent bitmap granularity with raw vols
On 06/24/2015 05:33 PM, Peter Krempa wrote: On Wed, Jun 24, 2015 at 16:48:16 +0800, Shanzhi Yu wrote: On 06/24/2015 03:11 PM, Peter Krempa wrote: qemu returns error but only in the event after the block job actually starts. Reject it upfront for a better error message. Instead of: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: Block Copy unexpectedly failed You will now get: $ virsh blockcopy vm hdc /tmp/raw.img --granularity 4096 --verbose --wait error: unsupported configuration: granularity can't be used with target volume format This can't avoid the cases that the source file has a raw format file as backing file. AFAIK if the active image (top layer) is a qcow2, then the destination is also created as a qcow2 and thus the persistent bitmap granularity should work just fine. Consider I have a guest as below: # virsh dumpxml r7-raw|grep disk -A 8 disk type='file' device='disk' driver name='qemu' type='qcow2'/ source file='/var/lib/libvirt/images/rhel7-raw.s1'/ backingStore type='file' index='1' format type='raw'/ source file='/var/lib/libvirt/images/rhel7-raw.img'/ backingStore/ /backingStore target dev='vda' bus='virtio'/ case I: # virsh blockcopy r7-raw vda /var/lib/libvirt/images/rhel7-raw.s1.cp --verbose --wait --granularity 4096 error: Block Copy unexpectedly failed case II: # virsh blockcopy r7-raw vda /var/lib/libvirt/images/rhel7-raw.s1.cp --verbose --wait --granularity 4096 --shallow Block Copy: [ 0 %]error: Block Copy unexpectedly failed Peter -- Regards shyu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] vz: Fix error messages in libvirt log caused by unhadled events
On 23.06.2015 13:26, Mikhail Feoktistov wrote: If the configuration of the instance has been modified, for example added disk or network device, then hypervisor sends event with prlIssuerType = PIE_DISPATCHER and EventType = PET_DSP_EVT_VM_CONFIG_CHANGED We should handle this event in prlsdkHandleVmEvent function to update instance's XML config. prlsdkHandleVmEvent is a common handler and it recieves many events. We don't need to handle all of them. Remove error message in case of unhandled events. --- src/vz/vz_sdk.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 98f7a57..d4d48e8 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1736,8 +1736,7 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) prlEvent = PRL_INVALID_HANDLE; break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping event type %d, prlEventType); } cleanup: @@ -1768,6 +1767,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) switch (prlIssuerType) { case PIE_VIRTUAL_MACHINE: +case PIE_DISPATCHER: prlsdkHandleVmEvent(privconn, prlEvent); // above function takes own of event prlEvent = PRL_INVALID_HANDLE; So as switch on issuer_type doesn't solve event discrimitation task probably it would be better just to move to previous scheme. Previous scheme has 2 very similar switches in prlsdkHandleVmEvent and prlsdkEventsHandler and now I understand why. 1. SDK hides 3 types behind PHT_EVENT handles. 1. IOEvent 2. IOEventPackage 3. VMEvent and only VMEvent has PrlEvent_GetIssuerId method, other types responds PRL_UNIMPLEMENTED. Next we need an issuer_id for all events of interest but we don't know if is safe to call PrlEvent_GetIssuerId as we have no means to discriminate IOEvent and VMEvent. So we discriminate by checking the result of PrlEvent_GetType instead. Thus the only purpose of first switch is to call PrlEvent_GetIssuerId safely. The better approach will probably be to ask for PrlEvent_GetIssuerId always and don't treat PRL_UNIMPLEMENTED as error. This way we can get rid of the first switch. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] docs: Properly mark acl.html dependencies
On Wed, Jun 24, 2015 at 08:53:21AM +0200, Peter Krempa wrote: On Tue, Jun 23, 2015 at 14:46:36 +0200, Martin Kletzander wrote: On Tue, Jun 23, 2015 at 01:56:00PM +0200, Michal Privoznik wrote: The acl.html file includes aclperms.htmlinc which is generated. However, acl.html is generated too from acl.html.tmp. And in fact, this is the place where the aclperms file is needed. Fix the dependency in Makefile. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- ACK (I thought I ACKed the previous one with this modification, I don't know why :). diff to v1: - Fix the origin of the error docs/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index b7b49cb..13dddf8 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -163,7 +163,7 @@ EXTRA_DIST= \ sitemap.html.in aclperms.htmlinc \ todo.pl hvsupport.pl todo.cfg-example -acl.html:: $(srcdir)/aclperms.htmlinc +acl.html.tmp: $(srcdir)/aclperms.htmlinc Now after a build I get: $ git status On branch master Your branch is up-to-date with 'origin/master'. Untracked files: (use git add file... to include in what will be committed) docs/acl.html.tmp nothing added to commit but untracked files present (use git add to track) Aha! I guess that's why the double colon must be there, because it sets one dependency and then there are bunch of other rules that I haven't found the first time (using wildcards), like '%.html: %.html.tmp' Weird that I don't see that after build... -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Use heads parameter for QXL driver
On Wed, Jun 24, 2015 at 06:34:50AM -0400, Frediano Ziglio wrote: On Fri, Jun 12, 2015 at 11:57:31AM +0100, Frediano Ziglio wrote: Allows to specify maximum number of head to QXL driver. Signed-off-by: Frediano Ziglio fzig...@redhat.com The patch to support the max_outputs in Qemu is still not merged but I got agreement on the name of the argument. Actually can be a compatiblity problem as heads in the XML configuration was set by default to '1'. Could we avoid that by passing the value to QEMU only if it's greater than 1? And what about if I just want one monitor ? Specify a one value ? Well, it probably depends on what the default is and how QEMU handles it. I guess we could just leave it as is. This controls only the maximum anyway, right? I think the client itself is supposed to control the actual number of displays, isn't it? --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.replies | 8 tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 8 tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 8 tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 8 tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 8 tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 8 tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.replies | 8 Why do you put it in outputs of all QEMU versions if it's not merged yet? To pass the checks That doesn't make sense. Each file is for each specific QEMU version. The .caps and .replies must match, but they don't all need to be the same. What would be the benefit then? Frediano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/7] Restore code to allow unpriv_sgio for hostdev SCSI generic
On Tue, Jun 16, 2015 at 03:51:23PM -0400, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1072736 This series of patches unreverts the functionality from commit id 'ce346623' which reverted the original functionality. What is the motivation to revert the revert? The commit message of that commit says: The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently. But 'git grep unpriv_sgio' gives me no results on linux-next, I only see it in the source of the dowstream kernel of a certain enterprise distro. If unpriv_sgio is RHEL-only, maybe we should revert it from upstream libvirt completely. Jan Since pure revert caused too many conflicts and because the code has undergone a few changes since the prior reversion, I had to restore the code by rote method. The reversion includes some refactorings to make the final check much easier to handle. Of note patch 3 covers much of what was adjusted in the reverted patch 'qemuCheckSharedDevice'. Patch 4 expands the driver lock to cover the same space as the similar disk code - I can take the other as well and shorten the time the disk code has the lock. Keeping them similar is mostly a sanity thing. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] scsi: Force error for SCSI pools on virStorageBackendSCSIFindLUs failure
On Tue, Jun 23, 2015 at 02:50:14PM -0400, John Ferlan wrote: Related to : https://bugzilla.redhat.com/show_bug.cgi?id=1171933 Rather than ignore the return status from virStorageBackendSCSIFindLUs, cause a failure to start the pool if a -1 is returned. Issue was noted during testing of the bz for iscsi that 'scsi' and 'fc' pools don't fail. Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_scsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vircapstest: Properly report error for failed tests
On Tue, Jun 23, 2015 at 01:39:52PM +0200, Michal Privoznik wrote: There are two macros used in the test: CAPSCOMP and CAPS_EXPECT_ERR. Both run a test case and if a failure occurred, they set the @ret variable to a value of -1 to indicate an error. Well, that's what they should do. Due to a typo, they set the variable to a positive one effectively masking any failed test. Then, we have couple of tests failing. Fix them too. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/vircapstest.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Address a couple of mpath/Multipath storage pool issues
My thought was to combine patch 2 and 3 unless it was felt that disallowing more than one Multipath pool to defined at one time (patch 3) was unnecessary For patch 1, I did some searching and came across the following to describe why /dev/mpath is no longer valid: http://comments.gmane.org/gmane.linux.kernel.device-mapper.devel/13864 I kept /dev/mpath as a valid option only just in case there's some strange back-compat issue or some platform where /dev/mpath is preferred/used instead of /dev/mapper. John Ferlan (3): mpath: Update path in CheckPool function docs: Clarify the Multipath target path value usage mpath: Don't allow more than one mpath pool at a time docs/formatstorage.html.in | 2 ++ docs/storage.html.in| 3 ++- src/conf/storage_conf.c | 3 +++ src/storage/storage_backend_mpath.c | 3 ++- 4 files changed, 9 insertions(+), 2 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] mpath: Update path in CheckPool function
https://bugzilla.redhat.com/show_bug.cgi?id=1230664 Per the devmapper docs, use /dev/mapper or /dev/dm-n in order to determine if a device is under control of DM Multipath. So add /dev/mapper to the virFileExists, leaving the /dev/mpath as a legacy option since it appears for a while it was the preferred mechanism, but is no longer maintained Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_mpath.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 971408a..ca9a62f 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -248,7 +248,8 @@ static int virStorageBackendMpathCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, bool *isActive) { -*isActive = virFileExists(/dev/mpath); +*isActive = virFileExists(/dev/mapper) || +virFileExists(/dev/mpath); return 0; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] mpath: Don't allow more than one mpath pool at a time
https://bugzilla.redhat.com/show_bug.cgi?id=1232606 Since an mpath pool contains all the Multipath devices on a host, allowing more than one defined on a host at a time should be disallowed under the policy of disallowing duplicate source pools for the host. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/storage.html.in| 3 ++- src/conf/storage_conf.c | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/storage.html.in b/docs/storage.html.in index 0b467d5..6c8222a 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -505,7 +505,8 @@ h2a name=StorageBackendMultipathMultipath pools/a/h2 p This provides a pool that contains all the multipath devices on the - host. Volume creating is not supported via the libvirt APIs. + host. Therefore, only one Multipath pool may be configured per host. + Volume creating is not supported via the libvirt APIs. The target element is actually ignored, but one is required to appease the libvirt XML parser.br/ br/ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4bbed4f..971f5c1 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2560,6 +2560,9 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, matchpool = pool; break; case VIR_STORAGE_POOL_MPATH: +/* Only one mpath pool is valid per host */ +matchpool = pool; +break; case VIR_STORAGE_POOL_RBD: case VIR_STORAGE_POOL_LAST: break; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] docs: Clarify the Multipath target path value usage
https://bugzilla.redhat.com/show_bug.cgi?id=1232606 As described in the storage.html.in, a Multipath pool ignores the target element in favor of the default target mapping of /dev/mapper. Indicate so for formatstorage.html.in as well. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatstorage.html.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index b6f4361..a60e05e 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -401,6 +401,8 @@ guaranteed stable across reboots, since they are allocated on demand. It is preferable to use a stable location such as one of the code/dev/disk/by-{path|id|uuid|label}/code locations. +For a Multipath pool (type codempath/code), the provided +value is ignored and the default value of /dev/mapper is used. span class=sinceSince 0.4.1/span /dd dtcodepermissions/code/dt -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: improve the way we format blkiotune and cputune
On 06/24/2015 05:32 PM, Martin Kletzander wrote: On Tue, Jun 23, 2015 at 09:24:25PM +0800, Luyao Huang wrote: Just refactor existing code to use a child buf instead of check all element before format blkiotune and cputune. This will avoid the more and more bigger element check during we introduce new elements in blkiotune and cputune in the future. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 168 +++-- 1 file changed, 65 insertions(+), 103 deletions(-) Nice cleanup, ACK Pushed. Thanks for your quick review Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: fix rules for paths with trailing slash
On Wed, Jun 24, 2015 at 09:58:45AM +0200, Cédric Bosdonnat wrote: Rules generated for a path like '/' were having '//' which isn't correct for apparmor. Make virt-aa-helper smarter to avoid these. --- src/security/virt-aa-helper.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 35423b5..9f1c570 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -773,6 +773,7 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi char *tmp = NULL; int rc = -1; bool readonly = true; +bool trailingSlash; if (path == NULL) return rc; @@ -809,14 +810,18 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi goto cleanup; } -virBufferAsprintf(buf, \%s%s\ %s,\n, tmp, recursive ? /** : , perms); +trailingSlash = (tmp[strlen(tmp) - 1] == '/'); Removing the trailing slash here and adding it unconditionally to the virBufferAsprintf formatting strings would be more readable. Jan + +virBufferAsprintf(buf, \%s%s%s\ %s,\n, tmp, trailingSlash ? : /, + recursive ? ** : , perms); if (readonly) { virBufferAddLit(buf, # don't audit writes to readonly files\n); -virBufferAsprintf(buf, deny \%s%s\ w,\n, tmp, recursive ? /** : ); +virBufferAsprintf(buf, deny \%s%s%s\ w,\n, tmp, + trailingSlash ? : /, recursive ? ** : ); } if (recursive) { /* allow reading (but not creating) the dir */ -virBufferAsprintf(buf, \%s/\ r,\n, tmp); +virBufferAsprintf(buf, \%s%s\ r,\n, tmp, trailingSlash ? : /); } cleanup: -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test: Refactor vcpu pinning and vcpu info retrieval
On 06/17/2015 11:51 AM, Peter Krempa wrote: Drop internal data structures and use the proper fields in virDomainDef. This allows to greatly simplify the code and allows to remove the private data structure that was holding just redundant data. This patch also fixes the bogous output where we'd report that a fresh VM without vCPU pinning would not run on all vcpus. --- This applies on top of [PATCH 0/8] Test driver refactors and fixes: src/test/test_driver.c | 224 +++-- 1 file changed, 49 insertions(+), 175 deletions(-) Just a note so it's not in someone's review list.. Replaced by patch 14 of v2 series... http://www.redhat.com/archives/libvir-list/2015-June/msg01249.html John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 14/15] test: Refactor vcpu pinning and vcpu info retrieval
On 06/24/2015 10:11 AM, Peter Krempa wrote: Drop internal data structures and use the proper fields in virDomainDef. This allows to greatly simplify the code and allows to remove the private data structure that was holding just redundant data. This patch also fixes the bogous output where we'd report that a fresh Another bogus typo VM without vCPU pinning would not run on all vcpus. --- src/test/test_driver.c | 224 +++-- 1 file changed, 49 insertions(+), 175 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ae332ef..ed67dca 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c ... @@ -2541,45 +2416,47 @@ static int testDomainGetVcpus(virDomainPtr domain, statbase = (tv.tv_sec * 1000UL * 1000UL) + tv.tv_usec; - hostcpus = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo); maxcpu = maplen * 8; if (maxcpu hostcpus) maxcpu = hostcpus; +if (!(allcpumap = virBitmapNew(hostcpus))) +goto cleanup; + +virBitmapSetAll(allcpumap); + /* Clamp to actual number of vcpus */ if (maxinfo privdom-def-vcpus) maxinfo = privdom-def-vcpus; -/* Populate virVcpuInfo structures */ -if (info != NULL) { -memset(info, 0, sizeof(*info) * maxinfo); +memset(info, 0, sizeof(*info) * maxinfo); +memset(cpumaps, 0, maxinfo * maplen); -for (i = 0; i maxinfo; i++) { -virVcpuInfo privinfo = privdomdata-vcpu_infos[i]; +for (i = 0; i maxinfo; i++) { +virDomainPinDefPtr pininfo; +virBitmapPtr bitmap = NULL; -info[i].number = privinfo.number; -info[i].state = privinfo.state; -info[i].cpu = privinfo.cpu; +pininfo = virDomainPinFind(def-cputune.vcpupin, + def-cputune.nvcpupin, + i); -/* Fake an increasing cpu time value */ -info[i].cpuTime = statbase / 10; -} -} +if (pininfo pininfo-cpumask) +bitmap = pininfo-cpumask; +else if (def-cpumask) +bitmap = def-cpumask; +else +bitmap = allcpumap; -/* Populate cpumaps */ -if (cpumaps != NULL) { -int privmaplen = VIR_CPU_MAPLEN(hostcpus); -memset(cpumaps, 0, maplen * maxinfo); +if (cpumaps) +virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i), maplen); -for (v = 0; v maxinfo; v++) { -unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); +info[i].number = i; +info[i].state = VIR_VCPU_RUNNING; +info[i].cpu = virBitmapLastSetBit(bitmap); -for (i = 0; i maxcpu; i++) { -if (VIR_CPU_USABLE(privdomdata-cpumaps, privmaplen, v, i)) -VIR_USE_CPU(cpumap, i); -} -} +/* Fake an increasing cpu time value */ +info[i].cpuTime = statbase / 10; } ret = maxinfo; ... Coverity determines that there's a need for virBitmapFree(allcpumap); John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 00/15] test driver refactors
On 06/24/2015 10:11 AM, Peter Krempa wrote: Version 2 has a few more fixes as suggested by Michal. Peter Krempa (15): test: Rename testConn to testDriver test: Drop useless forward declaration test: turn 'defaultConn' into a pointer test: Extract code to free testDriver into testDriverFree test: Extract common parts of test driver data allocation test: Drop unused attribute @path from testDriver struct test: Annotate few fields of testDriver structure test: Use atomic access to @nextDomID in struct virTestDriver test: Refactor test driver event sending test: Finalize removal of locking from driver-eventState test: Drop locked access to testDriver-domains test: Refactor test driver domain object retrieval test: Refactor testDomainSetVcpusFlags test: Refactor vcpu pinning and vcpu info retrieval test: Refactor testNodeGetCPUMap src/test/test_driver.c | 1305 +++- 1 file changed, 401 insertions(+), 904 deletions(-) Other than a couple of things pointed out things seem reasonable to me ACK John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/7] Restore code to allow unpriv_sgio for hostdev SCSI generic
On 06/24/2015 08:24 AM, Ján Tomko wrote: On Tue, Jun 16, 2015 at 03:51:23PM -0400, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1072736 This series of patches unreverts the functionality from commit id 'ce346623' which reverted the original functionality. What is the motivation to revert the revert? The commit message of that commit says: The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently. But 'git grep unpriv_sgio' gives me no results on linux-next, I only see it in the source of the dowstream kernel of a certain enterprise distro. If unpriv_sgio is RHEL-only, maybe we should revert it from upstream libvirt completely. Jan I guess it wasn't perfectly clear from the original change that unpriv_sgio was only for one downstream implementation and only for shared hostdev (generic scsi), but now that I read the related cases with this in mind, I see your point. I suppose there could be reason to carry it only for that downstream source pool, although I think the overhead of doing that could be made easier by accepting these patches. At the very least a couple of the patches are not necessarily sgio hostdev related. Perhaps only patch 6 would need to be reworked to remove the hostdev_path and patch 7 could be removed. It seems from my reading that sgio is OK for disk since that's not the contention here, correct? Or does that need to be removed as well? John Since pure revert caused too many conflicts and because the code has undergone a few changes since the prior reversion, I had to restore the code by rote method. The reversion includes some refactorings to make the final check much easier to handle. Of note patch 3 covers much of what was adjusted in the reverted patch 'qemuCheckSharedDevice'. Patch 4 expands the driver lock to cover the same space as the similar disk code - I can take the other as well and shorten the time the disk code has the lock. Keeping them similar is mostly a sanity thing. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/15] test: turn 'defaultConn' into a pointer
On Wed, Jun 24, 2015 at 04:11:45PM +0200, Peter Krempa wrote: --- src/test/test_driver.c | 46 -- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 8d2bb5b..ff383c6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -117,7 +117,7 @@ struct _testDriver { typedef struct _testDriver testDriver; typedef testDriver *testDriverPtr; -static testDriver defaultConn; +static testDriverPtr defaultConn; The variable could be also renamed to defaultDrv. static int defaultConnections; static virMutex defaultLock = VIR_MUTEX_INITIALIZER; @@ -691,7 +691,7 @@ static int testOpenDefault(virConnectPtr conn) { int u; -testDriverPtr privconn = defaultConn; +testDriverPtr privconn = NULL; Same here, this could be privdrv. virDomainDefPtr domdef = NULL; virDomainObjPtr domobj = NULL; virNetworkDefPtr netdef = NULL; @@ -705,17 +705,18 @@ testOpenDefault(virConnectPtr conn) virMutexLock(defaultLock); if (defaultConnections++) { -conn-privateData = defaultConn; +conn-privateData = defaultConn; virMutexUnlock(defaultLock); return VIR_DRV_OPEN_SUCCESS; } +if (VIR_ALLOC(privconn) 0) +goto error; + if (virMutexInit(privconn-lock) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot initialize mutex)); -defaultConnections--; -virMutexUnlock(defaultLock); -return VIR_DRV_OPEN_ERROR; +goto error; } conn-privateData = privconn; @@ -822,19 +823,23 @@ testOpenDefault(virConnectPtr conn) } virNodeDeviceObjUnlock(nodeobj); +defaultConn = privconn; + virMutexUnlock(defaultLock); return VIR_DRV_OPEN_SUCCESS; error: -virObjectUnref(privconn-domains); -virObjectUnref(privconn-networks); -virInterfaceObjListFree(privconn-ifaces); -virStoragePoolObjListFree(privconn-pools); -virNodeDeviceObjListFree(privconn-devs); -virObjectUnref(privconn-caps); -virObjectEventStateFree(privconn-eventState); -virMutexDestroy(privconn-lock); +if (privconn) { +virObjectUnref(privconn-domains); +virObjectUnref(privconn-networks); +virInterfaceObjListFree(privconn-ifaces); +virStoragePoolObjListFree(privconn-pools); +virNodeDeviceObjListFree(privconn-devs); +virObjectUnref(privconn-caps); +virObjectEventStateFree(privconn-eventState); +virMutexDestroy(privconn-lock); +} conn-privateData = NULL; virDomainDefFree(domdef); defaultConnections--; @@ -1573,8 +1578,10 @@ static virDrvOpenStatus testConnectOpen(virConnectPtr conn, static int testConnectClose(virConnectPtr conn) { testDriverPtr privconn = conn-privateData; +bool dflt = false; -if (privconn == defaultConn) { +if (privconn == defaultConn) { +dflt = true; virMutexLock(defaultLock); if (--defaultConnections) { virMutexUnlock(defaultLock); @@ -1596,10 +1603,13 @@ static int testConnectClose(virConnectPtr conn) testDriverUnlock(privconn); virMutexDestroy(privconn-lock); -if (privconn == defaultConn) +VIR_FREE(privconn); + +if (dflt) { +defaultConn = NULL; virMutexUnlock(defaultLock); -else -VIR_FREE(privconn); +} + conn-privateData = NULL; return 0; } -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] vz: implement connection close notification
Reuse virConnectCloseCallback to implement connection close event functions. Thus we automatically meet multi-thread requirements on unregistering/notification. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- daemon/remote.c|2 +- src/remote/remote_driver.c |3 ++- src/vz/vz_driver.c | 26 ++ src/vz/vz_sdk.c| 32 ++-- src/vz/vz_utils.h |3 +++ 5 files changed, 62 insertions(+), 4 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index e0cc008..ca4b63d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1198,7 +1198,7 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int r remote_connect_event_connection_closed_msg msg = { reason }; remoteDispatchObjectEventSend(client, remoteProgram, - REMOTE_PROC_CONNECT_CLOSE, + REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED, (xdrproc_t)xdr_remote_connect_event_connection_closed_msg, msg); } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e6236be..90f813a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -522,9 +522,10 @@ remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog ATTRIBUTE_U void *evdata, void *opaque) { virConnectPtr conn = opaque; +struct private_data *priv = conn-privateData; remote_connect_event_connection_closed_msg *msg = evdata; -virConnectCloseCallbackCall(conn-privateData, msg-reason); +virConnectCloseCallbackCall(priv-closeCallback, msg-reason); } static void diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..26caf19 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -268,6 +268,9 @@ vzOpenDefault(virConnectPtr conn) if (prlsdkSubscribeToPCSEvents(privconn)) goto error; +if (!(privconn-closeCallback = virGetConnectCloseCallback())) +goto error; + conn-privateData = privconn; if (prlsdkLoadDomains(privconn)) @@ -276,6 +279,8 @@ vzOpenDefault(virConnectPtr conn) return VIR_DRV_OPEN_SUCCESS; error: +virObjectUnref(privconn-closeCallback); +privconn-closeCallback = NULL; virObjectUnref(privconn-domains); virObjectUnref(privconn-caps); virStoragePoolObjListFree(privconn-pools); @@ -350,6 +355,8 @@ vzConnectClose(virConnectPtr conn) virObjectUnref(privconn-caps); virObjectUnref(privconn-xmlopt); virObjectUnref(privconn-domains); +virObjectUnref(privconn-closeCallback); +privconn-closeCallback = NULL; virObjectEventStateFree(privconn-domainEventState); prlsdkDisconnect(privconn); conn-privateData = NULL; @@ -1321,6 +1328,23 @@ vzDomainBlockStatsFlags(virDomainPtr domain, return ret; } +static int +vzConnectRegisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ +vzConnPtr privconn = conn-privateData; +return virConnectCloseCallbackRegister(privconn-closeCallback, conn, cb, opaque, freecb); +} + +static int +vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb ATTRIBUTE_UNUSED) +{ +vzConnPtr privconn = conn-privateData; +virConnectCloseCallbackUnregister(privconn-closeCallback); +return 0; +} static virHypervisorDriver vzDriver = { .name = vz, @@ -1373,6 +1397,8 @@ static virHypervisorDriver vzDriver = { .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */ .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */ .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */ +.connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.0 */ +.connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.0 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 98f7a57..fef064c 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1736,8 +1736,7 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) prlEvent = PRL_INVALID_HANDLE; break; default: -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Can't handle event of type %d), prlEventType); +VIR_DEBUG(Skipping vm event of type %d, prlEventType); } cleanup: @@ -1745,6 +1744,32 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) return; } +static +void prlsdkHandleDispatcherConnectionClosed(vzConnPtr privconn) +{ +virConnectCloseCallbackCall(privconn-closeCallback, VIR_CONNECT_CLOSE_REASON_EOF); +} + +static void +prlsdkHandleDispatcherEvent(vzConnPtr privconn, PRL_HANDLE prlEvent) +{ +PRL_RESULT pret
[libvirt] driver level connection close event
Notify of connection close event from parallels driver (possibly) wrapped in the remote driver. Discussion. In 1 and 2 patch we forced to some decisions because we don't have a weak reference mechanics. 1 patch. --- virConnectCloseCallback is introduced because we can not reference the connection object itself when setting a network layer callback because of how connection close works. A connection close procedure is next: 1. client closes connection 2. a this point nobody else referencing a connection and it is disposed 3. connection dispose unreferencing network connection 4. network connection disposes Thus if we referece a connection in network close callback we never get step 2. virConnectCloseCallback broke this cycle but at cost that clients MUST unregister explicitly before closing connection. This is not good as this unregistration is not really neaded. Client is not telling that it does not want to receive events anymore but rather forced to obey some implementation-driven rules. 2 patch. --- We impose requirements on driver implementations which is fragile. Moreover we again need to make explicit unregistrations. Implementation of domain events illustrates this point. remoteDispatchConnectDomainEventRegister does not reference NetClient and makes unregistration before NetClient is disposed but drivers do not meet the formulated requirements. Object event system release lock before delivering event for re-entrance purposes. Shortly we have 2 undesired consequences here. 1. Mandatory unregistration. 2. Imposing multi-threading requirements. Introduction of weak pointers could free us from these artifacts. Next weak reference workflow illustrates this. 1. Take weak reference on object of interest before passing to party. This doesn't break disposing mechanics as weak eference does not prevent from disposing object. Object is disposed but memory is not freed yet if there are weak references. 2. When callback is called we are safe to check if pointer dangling as we make a weak reference before. 3. Release weak reference and this trigger memory freeing if there are no more weak references. daemon/libvirtd.h|1 + daemon/remote.c | 87 src/datatypes.c | 112 +++-- src/datatypes.h | 21 ++-- src/driver-hypervisor.h | 12 + src/libvirt-host.c | 79 ++ src/remote/remote_driver.c | 108 +++- src/remote/remote_protocol.x | 24 +- src/remote_protocol-structs |6 ++ src/vz/vz_driver.c | 26 ++ src/vz/vz_sdk.c | 32 +++- src/vz/vz_utils.h|3 + 12 files changed, 396 insertions(+), 115 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/15] test: turn 'defaultConn' into a pointer
On Wed, Jun 24, 2015 at 16:33:37 +0200, Pavel Hrdina wrote: On Wed, Jun 24, 2015 at 04:11:45PM +0200, Peter Krempa wrote: --- src/test/test_driver.c | 46 -- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 8d2bb5b..ff383c6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -117,7 +117,7 @@ struct _testDriver { typedef struct _testDriver testDriver; typedef testDriver *testDriverPtr; -static testDriver defaultConn; +static testDriverPtr defaultConn; The variable could be also renamed to defaultDrv. Patches are welcome ;) Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/15] test: Rename testConn to testDriver
--- src/test/test_driver.c | 308 - 1 file changed, 154 insertions(+), 154 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6613ed7..b1dca29 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -92,7 +92,7 @@ struct _testAuth { typedef struct _testAuth testAuth; typedef struct _testAuth *testAuthPtr; -struct _testConn { +struct _testDriver { virMutex lock; char *path; @@ -114,10 +114,10 @@ struct _testConn { virObjectEventStatePtr eventState; }; -typedef struct _testConn testConn; -typedef testConn *testConnPtr; +typedef struct _testDriver testDriver; +typedef testDriver *testDriverPtr; -static testConn defaultConn; +static testDriver defaultConn; static int defaultConnections; static virMutex defaultLock = VIR_MUTEX_INITIALIZER; @@ -137,15 +137,15 @@ static const virNodeInfo defaultNodeInfo = { static int testConnectClose(virConnectPtr conn); -static void testObjectEventQueue(testConnPtr driver, +static void testObjectEventQueue(testDriverPtr driver, virObjectEventPtr event); -static void testDriverLock(testConnPtr driver) +static void testDriverLock(testDriverPtr driver) { virMutexLock(driver-lock); } -static void testDriverUnlock(testConnPtr driver) +static void testDriverUnlock(testDriverPtr driver) { virMutexUnlock(driver-lock); } @@ -311,7 +311,7 @@ testBuildXMLConfig(void) static virCapsPtr testBuildCapabilities(virConnectPtr conn) { -testConnPtr privconn = conn-privateData; +testDriverPtr privconn = conn-privateData; virCapsPtr caps; virCapsGuestPtr guest; int guest_types[] = { VIR_DOMAIN_OSTYPE_HVM, @@ -480,7 +480,7 @@ static virDomainObjPtr testDomObjFromDomain(virDomainPtr domain) { virDomainObjPtr vm; -testConnPtr driver = domain-conn-privateData; +testDriverPtr driver = domain-conn-privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; testDriverLock(driver); @@ -601,7 +601,7 @@ testDomainUpdateVCPU(virDomainObjPtr dom, * @clear_all: If true, rebuild info for ALL vcpus, not just newly added vcpus */ static int -testDomainUpdateVCPUs(testConnPtr privconn, +testDomainUpdateVCPUs(testDriverPtr privconn, virDomainObjPtr dom, int nvcpus, unsigned int clear_all) @@ -658,7 +658,7 @@ testDomainShutdownState(virDomainPtr domain, /* Set up domain runtime state */ static int -testDomainStartState(testConnPtr privconn, +testDomainStartState(testDriverPtr privconn, virDomainObjPtr dom, virDomainRunningReason reason) { @@ -692,7 +692,7 @@ static int testOpenDefault(virConnectPtr conn) { int u; -testConnPtr privconn = defaultConn; +testDriverPtr privconn = defaultConn; virDomainDefPtr domdef = NULL; virDomainObjPtr domobj = NULL; virNetworkDefPtr netdef = NULL; @@ -1002,7 +1002,7 @@ testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) } static int -testParseDomainSnapshots(testConnPtr privconn, +testParseDomainSnapshots(testDriverPtr privconn, virDomainObjPtr domobj, const char *file, xmlXPathContextPtr ctxt) @@ -1058,7 +1058,7 @@ testParseDomainSnapshots(testConnPtr privconn, } static int -testParseDomains(testConnPtr privconn, +testParseDomains(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { @@ -1123,7 +1123,7 @@ testParseDomains(testConnPtr privconn, } static int -testParseNetworks(testConnPtr privconn, +testParseNetworks(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { @@ -1162,7 +1162,7 @@ testParseNetworks(testConnPtr privconn, } static int -testParseInterfaces(testConnPtr privconn, +testParseInterfaces(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { @@ -1258,7 +1258,7 @@ testOpenVolumesForPool(const char *file, } static int -testParseStorage(testConnPtr privconn, +testParseStorage(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { @@ -1310,7 +1310,7 @@ testParseStorage(testConnPtr privconn, } static int -testParseNodedevs(testConnPtr privconn, +testParseNodedevs(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { @@ -1349,7 +1349,7 @@ testParseNodedevs(testConnPtr privconn, } static int -testParseAuthUsers(testConnPtr privconn, +testParseAuthUsers(testDriverPtr privconn, xmlXPathContextPtr ctxt) { int num, ret = -1; @@ -1395,7 +1395,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; -testConnPtr privconn; +testDriverPtr
[libvirt] [PATCH v2 08/15] test: Use atomic access to @nextDomID in struct virTestDriver
--- src/test/test_driver.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0cd8e6a..1d54639 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -59,6 +59,7 @@ #include virstring.h #include cpu/cpu.h #include virauth.h +#include viratomic.h #define VIR_FROM_THIS VIR_FROM_TEST @@ -95,7 +96,6 @@ typedef struct _testAuth *testAuthPtr; struct _testDriver { virMutex lock; -int nextDomID; virNodeInfo nodeInfo; virInterfaceObjList ifaces; bool transaction_running; @@ -107,6 +107,9 @@ struct _testDriver { size_t numAuths; testAuthPtr auths; +/* virAtomic access only */ +volatile int nextDomID; + /* immutable pointer, immutable object after being initialized with * testBuildCapabilities */ virCapsPtr caps; @@ -417,7 +420,7 @@ testDriverNew(void) !(ret-networks = virNetworkObjListNew())) goto error; -ret-nextDomID = 1; +virAtomicIntSet(ret-nextDomID, 1); return ret; @@ -712,7 +715,7 @@ testDomainStartState(testDriverPtr privconn, goto cleanup; virDomainObjSetState(dom, VIR_DOMAIN_RUNNING, reason); -dom-def-id = privconn-nextDomID++; +dom-def-id = virAtomicIntAdd(privconn-nextDomID, 1); if (virDomainObjSetDefTransient(privconn-caps, privconn-xmlopt, -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 13/15] test: Refactor testDomainSetVcpusFlags
Remove the bogous flag check and refactor the code by using virDomainObjGetDefs instead of virDomainObjGetPersistentDef. --- src/test/test_driver.c | 68 +- 1 file changed, 23 insertions(+), 45 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2c2f009..ae332ef 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2445,6 +2445,7 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, { testDriverPtr privconn = domain-conn-privateData; virDomainObjPtr privdom = NULL; +virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1, maxvcpus; @@ -2452,72 +2453,49 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); -/* At least one of LIVE or CONFIG must be set. MAXIMUM cannot be - * mixed with LIVE. */ -if ((flags (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == 0 || -(flags (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) == - (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) { -virReportError(VIR_ERR_INVALID_ARG, - _(invalid flag combination: (0x%x)), flags); +if ((maxvcpus = testConnectGetMaxVcpus(domain-conn, NULL)) == -1) return -1; -} -if (!nrCpus || (maxvcpus = testConnectGetMaxVcpus(domain-conn, NULL)) nrCpus) { + +if (nrCpus maxvcpus) { virReportError(VIR_ERR_INVALID_ARG, - _(argument out of range: %d), nrCpus); + _(requested cpu amount exceeds maximum supported amount + (%d %d)), nrCpus, maxvcpus); return -1; } if (!(privdom = testDomObjFromDomain(domain))) return -1; -if (!virDomainObjIsActive(privdom) (flags VIR_DOMAIN_AFFECT_LIVE)) { -virReportError(VIR_ERR_OPERATION_INVALID, - %s, _(cannot hotplug vcpus for an inactive domain)); +if (virDomainObjGetDefs(privdom, flags, def, persistentDef) 0) goto cleanup; -} - -/* We allow more cpus in guest than host, but not more than the - * domain's starting limit. */ -if (!(flags (VIR_DOMAIN_VCPU_MAXIMUM)) -privdom-def-maxvcpus maxvcpus) -maxvcpus = privdom-def-maxvcpus; -if (nrCpus maxvcpus) { +if ((def + def-maxvcpus nrCpus) || +(persistentDef + !(flags VIR_DOMAIN_VCPU_MAXIMUM) + persistentDef-maxvcpus nrCpus)) { virReportError(VIR_ERR_INVALID_ARG, _(requested cpu amount exceeds maximum (%d %d)), nrCpus, maxvcpus); goto cleanup; } -if (!(persistentDef = virDomainObjGetPersistentDef(privconn-caps, - privconn-xmlopt, - privdom))) +if (def +testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0) 0) goto cleanup; -switch (flags) { -case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_CONFIG: -persistentDef-maxvcpus = nrCpus; -if (nrCpus persistentDef-vcpus) -persistentDef-vcpus = nrCpus; -ret = 0; -break; - -case VIR_DOMAIN_AFFECT_CONFIG: -persistentDef-vcpus = nrCpus; -ret = 0; -break; - -case VIR_DOMAIN_AFFECT_LIVE: -ret = testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0); -break; - -case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG: -ret = testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0); -if (ret == 0) +if (persistentDef) { +if (flags VIR_DOMAIN_VCPU_MAXIMUM) { +persistentDef-maxvcpus = nrCpus; +if (nrCpus persistentDef-vcpus) +persistentDef-vcpus = nrCpus; +} else { persistentDef-vcpus = nrCpus; -break; +} } +ret = 0; + cleanup: virDomainObjEndAPI(privdom); return ret; -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 10/15] test: Finalize removal of locking from driver-eventState
Don't lock the driver when registering event callbacks. --- src/test/test_driver.c | 12 1 file changed, 12 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2ab0402..b08c1e5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5915,11 +5915,9 @@ testConnectDomainEventRegister(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virDomainEventStateRegister(conn, driver-eventState, callback, opaque, freecb) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5932,11 +5930,9 @@ testConnectDomainEventDeregister(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virDomainEventStateDeregister(conn, driver-eventState, callback) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5953,12 +5949,10 @@ testConnectDomainEventRegisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret; -testDriverLock(driver); if (virDomainEventStateRegisterID(conn, driver-eventState, dom, eventID, callback, opaque, freecb, ret) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5970,11 +5964,9 @@ testConnectDomainEventDeregisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virObjectEventStateDeregisterID(conn, driver-eventState, callbackID) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5991,12 +5983,10 @@ testConnectNetworkEventRegisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret; -testDriverLock(driver); if (virNetworkEventStateRegisterID(conn, driver-eventState, net, eventID, callback, opaque, freecb, ret) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -6008,11 +5998,9 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virObjectEventStateDeregisterID(conn, driver-eventState, callbackID) 0) ret = -1; -testDriverUnlock(driver); return ret; } -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/15] test: Extract code to free testDriver into testDriverFree
Avoid reimplementing it 3 times. --- src/test/test_driver.c | 55 +- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ff383c6..732dde9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -135,6 +135,26 @@ static const virNodeInfo defaultNodeInfo = { 2, }; +static void +testDriverFree(testDriverPtr driver) +{ +if (!driver) +return; + +virObjectUnref(driver-caps); +virObjectUnref(driver-xmlopt); +virObjectUnref(driver-domains); +virNodeDeviceObjListFree(driver-devs); +virObjectUnref(driver-networks); +virInterfaceObjListFree(driver-ifaces); +virStoragePoolObjListFree(driver-pools); +virObjectEventStateFree(driver-eventState); +VIR_FREE(driver-path); +virMutexUnlock(driver-lock); +virMutexDestroy(driver-lock); + +VIR_FREE(driver); +} static void testObjectEventQueue(testDriverPtr driver, virObjectEventPtr event); @@ -830,16 +850,7 @@ testOpenDefault(virConnectPtr conn) return VIR_DRV_OPEN_SUCCESS; error: -if (privconn) { -virObjectUnref(privconn-domains); -virObjectUnref(privconn-networks); -virInterfaceObjListFree(privconn-ifaces); -virStoragePoolObjListFree(privconn-pools); -virNodeDeviceObjListFree(privconn-devs); -virObjectUnref(privconn-caps); -virObjectEventStateFree(privconn-eventState); -virMutexDestroy(privconn-lock); -} +testDriverFree(privconn); conn-privateData = NULL; virDomainDefFree(domdef); defaultConnections--; @@ -1465,14 +1476,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) error: xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); -virObjectUnref(privconn-domains); -virObjectUnref(privconn-networks); -virInterfaceObjListFree(privconn-ifaces); -virStoragePoolObjListFree(privconn-pools); -VIR_FREE(privconn-path); -virObjectEventStateFree(privconn-eventState); -testDriverUnlock(privconn); -VIR_FREE(privconn); +testDriverFree(privconn); conn-privateData = NULL; return VIR_DRV_OPEN_ERROR; } @@ -1590,20 +1594,7 @@ static int testConnectClose(virConnectPtr conn) } testDriverLock(privconn); -virObjectUnref(privconn-caps); -virObjectUnref(privconn-xmlopt); -virObjectUnref(privconn-domains); -virNodeDeviceObjListFree(privconn-devs); -virObjectUnref(privconn-networks); -virInterfaceObjListFree(privconn-ifaces); -virStoragePoolObjListFree(privconn-pools); -virObjectEventStateFree(privconn-eventState); -VIR_FREE(privconn-path); - -testDriverUnlock(privconn); -virMutexDestroy(privconn-lock); - -VIR_FREE(privconn); +testDriverFree(privconn); if (dflt) { defaultConn = NULL; -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/15] test: Drop unused attribute @path from testDriver struct
It's filled and then freed, but not used anywhere else. --- src/test/test_driver.c | 4 1 file changed, 4 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c68b3d6..f105055 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -95,7 +95,6 @@ typedef struct _testAuth *testAuthPtr; struct _testDriver { virMutex lock; -char *path; int nextDomID; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; @@ -149,7 +148,6 @@ testDriverFree(testDriverPtr driver) virInterfaceObjListFree(driver-ifaces); virStoragePoolObjListFree(driver-pools); virObjectEventStateFree(driver-eventState); -VIR_FREE(driver-path); virMutexUnlock(driver-lock); virMutexDestroy(driver-lock); @@ -1434,8 +1432,6 @@ testOpenFromFile(virConnectPtr conn, const char *file) } privconn-numCells = 0; -if (VIR_STRDUP(privconn-path, file) 0) -goto error; memmove(privconn-nodeInfo, defaultNodeInfo, sizeof(defaultNodeInfo)); if (testParseNodeInfo(privconn-nodeInfo, ctxt) 0) -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 11/15] test: Drop locked access to testDriver-domains
Only self-locking APIs are used and the pointer is immutable so there's no need to lock the driver to access the domain list. This patch removes locking partially for everything that will not be converted to testDomObjFromDomain in the next patch. --- src/test/test_driver.c | 73 +++--- 1 file changed, 10 insertions(+), 63 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b08c1e5..c36e0f8 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -537,7 +537,6 @@ testDomObjFromDomain(virDomainPtr domain) testDriverPtr driver = domain-conn-privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; -testDriverLock(driver); vm = virDomainObjListFindByUUIDRef(driver-domains, domain-uuid); if (!vm) { virUUIDFormat(domain-uuid, uuidstr); @@ -546,7 +545,6 @@ testDomObjFromDomain(virDomainPtr domain) uuidstr, domain-name); } -testDriverUnlock(driver); return vm; } @@ -1790,11 +1788,7 @@ static virDomainPtr testDomainLookupByID(virConnectPtr conn, virDomainPtr ret = NULL; virDomainObjPtr dom; -testDriverLock(privconn); -dom = virDomainObjListFindByID(privconn-domains, id); -testDriverUnlock(privconn); - -if (dom == NULL) { +if (!(dom = virDomainObjListFindByID(privconn-domains, id))) { virReportError(VIR_ERR_NO_DOMAIN, NULL); goto cleanup; } @@ -1816,11 +1810,7 @@ static virDomainPtr testDomainLookupByUUID(virConnectPtr conn, virDomainPtr ret = NULL; virDomainObjPtr dom; -testDriverLock(privconn); -dom = virDomainObjListFindByUUID(privconn-domains, uuid); -testDriverUnlock(privconn); - -if (dom == NULL) { +if (!(dom = virDomainObjListFindByUUID(privconn-domains, uuid))) { virReportError(VIR_ERR_NO_DOMAIN, NULL); goto cleanup; } @@ -1842,11 +1832,7 @@ static virDomainPtr testDomainLookupByName(virConnectPtr conn, virDomainPtr ret = NULL; virDomainObjPtr dom; -testDriverLock(privconn); -dom = virDomainObjListFindByName(privconn-domains, name); -testDriverUnlock(privconn); - -if (dom == NULL) { +if (!(dom = virDomainObjListFindByName(privconn-domains, name))) { virReportError(VIR_ERR_NO_DOMAIN, NULL); goto cleanup; } @@ -1865,13 +1851,9 @@ static int testConnectListDomains(virConnectPtr conn, int maxids) { testDriverPtr privconn = conn-privateData; -int n; - -testDriverLock(privconn); -n = virDomainObjListGetActiveIDs(privconn-domains, ids, maxids, NULL, NULL); -testDriverUnlock(privconn); -return n; +return virDomainObjListGetActiveIDs(privconn-domains, ids, maxids, +NULL, NULL); } static int testDomainDestroy(virDomainPtr domain) @@ -1881,7 +1863,6 @@ static int testDomainDestroy(virDomainPtr domain) virObjectEventPtr event = NULL; int ret = -1; -testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn-domains, domain-name); @@ -1902,7 +1883,6 @@ static int testDomainDestroy(virDomainPtr domain) cleanup: virDomainObjEndAPI(privdom); testObjectEventQueue(privconn, event); -testDriverUnlock(privconn); return ret; } @@ -1989,7 +1969,6 @@ static int testDomainShutdownFlags(virDomainPtr domain, virCheckFlags(0, -1); -testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn-domains, domain-name); @@ -2016,7 +1995,6 @@ static int testDomainShutdownFlags(virDomainPtr domain, cleanup: virDomainObjEndAPI(privdom); testObjectEventQueue(privconn, event); -testDriverUnlock(privconn); return ret; } @@ -2034,7 +2012,6 @@ static int testDomainReboot(virDomainPtr domain, virObjectEventPtr event = NULL; int ret = -1; -testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn-domains, domain-name); @@ -2087,7 +2064,6 @@ static int testDomainReboot(virDomainPtr domain, cleanup: virDomainObjEndAPI(privdom); testObjectEventQueue(privconn, event); -testDriverUnlock(privconn); return ret; } @@ -2178,7 +2154,6 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, return -1; } -testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn-domains, domain-name); @@ -2252,7 +2227,6 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, } virDomainObjEndAPI(privdom); testObjectEventQueue(privconn, event); -testDriverUnlock(privconn); return ret; } @@ -2286,8 +2260,6 @@ testDomainRestoreFlags(virConnectPtr conn, return -1; } -testDriverLock(privconn); - if ((fd = open(path, O_RDONLY)) 0) {
[libvirt] [PATCH v2 07/15] test: Annotate few fields of testDriver structure
Some of the fields are either immutable or self locking, so make a note of that for future reference. --- src/test/test_driver.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f105055..0cd8e6a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -96,11 +96,7 @@ struct _testDriver { virMutex lock; int nextDomID; -virCapsPtr caps; -virDomainXMLOptionPtr xmlopt; virNodeInfo nodeInfo; -virDomainObjListPtr domains; -virNetworkObjListPtr networks; virInterfaceObjList ifaces; bool transaction_running; virInterfaceObjList backupIfaces; @@ -111,6 +107,16 @@ struct _testDriver { size_t numAuths; testAuthPtr auths; +/* immutable pointer, immutable object after being initialized with + * testBuildCapabilities */ +virCapsPtr caps; + +/* immutable pointer, immutable object */ +virDomainXMLOptionPtr xmlopt; + +/* immutable pointer, self-locking APIs */ +virDomainObjListPtr domains; +virNetworkObjListPtr networks; virObjectEventStatePtr eventState; }; typedef struct _testDriver testDriver; -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 05/15] test: Extract common parts of test driver data allocation
--- src/test/test_driver.c | 96 +- 1 file changed, 41 insertions(+), 55 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 732dde9..c68b3d6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -309,24 +309,6 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return -1; } -static virDomainXMLOptionPtr -testBuildXMLConfig(void) -{ -virDomainXMLPrivateDataCallbacks priv = { -.alloc = testDomainObjPrivateAlloc, -.free = testDomainObjPrivateFree -}; - -/* All our XML extensions are input only, so we only need to parse */ -virDomainXMLNamespace ns = { -.parse = testDomainDefNamespaceParse, -.free = testDomainDefNamespaceFree, -}; - -return virDomainXMLOptionNew(NULL, priv, ns); -} - - static virCapsPtr testBuildCapabilities(virConnectPtr conn) { @@ -402,6 +384,45 @@ testBuildCapabilities(virConnectPtr conn) } +static testDriverPtr +testDriverNew(void) +{ +virDomainXMLPrivateDataCallbacks priv = { +.alloc = testDomainObjPrivateAlloc, +.free = testDomainObjPrivateFree +}; + +virDomainXMLNamespace ns = { +.parse = testDomainDefNamespaceParse, +.free = testDomainDefNamespaceFree, +}; +testDriverPtr ret; + +if (VIR_ALLOC(ret) 0) +return NULL; + +if (virMutexInit(ret-lock) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + %s, _(cannot initialize mutex)); +goto error; +} + +if (!(ret-xmlopt = virDomainXMLOptionNew(NULL, priv, ns)) || +!(ret-eventState = virObjectEventStateNew()) || +!(ret-domains = virDomainObjListNew()) || +!(ret-networks = virNetworkObjListNew())) +goto error; + +ret-nextDomID = 1; + +return ret; + + error: +testDriverFree(ret); +return NULL; +} + + static const char *defaultDomainXML = domain type='test' nametest/name @@ -730,24 +751,11 @@ testOpenDefault(virConnectPtr conn) return VIR_DRV_OPEN_SUCCESS; } -if (VIR_ALLOC(privconn) 0) -goto error; - -if (virMutexInit(privconn-lock) 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - %s, _(cannot initialize mutex)); +if (!(privconn = testDriverNew())) goto error; -} conn-privateData = privconn; -if (!(privconn-eventState = virObjectEventStateNew())) -goto error; - -if (!(privconn-domains = virDomainObjListNew()) || -!(privconn-networks = virNetworkObjListNew())) -goto error; - memmove(privconn-nodeInfo, defaultNodeInfo, sizeof(defaultNodeInfo)); /* Numa setup */ @@ -770,11 +778,6 @@ testOpenDefault(virConnectPtr conn) if (!(privconn-caps = testBuildCapabilities(conn))) goto error; -if (!(privconn-xmlopt = testBuildXMLConfig())) -goto error; - -privconn-nextDomID = 1; - if (!(domdef = virDomainDefParseString(defaultDomainXML, privconn-caps, privconn-xmlopt, @@ -1412,31 +1415,15 @@ testOpenFromFile(virConnectPtr conn, const char *file) xmlXPathContextPtr ctxt = NULL; testDriverPtr privconn; -if (VIR_ALLOC(privconn) 0) -return VIR_DRV_OPEN_ERROR; -if (virMutexInit(privconn-lock) 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - %s, _(cannot initialize mutex)); -VIR_FREE(privconn); +if (!(privconn = testDriverNew())) return VIR_DRV_OPEN_ERROR; -} testDriverLock(privconn); conn-privateData = privconn; -if (!(privconn-domains = virDomainObjListNew()) || -!(privconn-networks = virNetworkObjListNew())) -goto error; - if (!(privconn-caps = testBuildCapabilities(conn))) goto error; -if (!(privconn-xmlopt = testBuildXMLConfig())) -goto error; - -if (!(privconn-eventState = virObjectEventStateNew())) -goto error; - if (!(doc = virXMLParseFileCtxt(file, ctxt))) goto error; @@ -1446,7 +1433,6 @@ testOpenFromFile(virConnectPtr conn, const char *file) goto error; } -privconn-nextDomID = 1; privconn-numCells = 0; if (VIR_STRDUP(privconn-path, file) 0) goto error; -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 02/15] test: Drop useless forward declaration
--- src/test/test_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b1dca29..8d2bb5b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -136,7 +136,6 @@ static const virNodeInfo defaultNodeInfo = { }; -static int testConnectClose(virConnectPtr conn); static void testObjectEventQueue(testDriverPtr driver, virObjectEventPtr event); -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/15] test: Refactor test driver event sending
Make testObjectEventQueue tolerant to NULL @event and move it so that it does not require a prototype. Additionally we are now able to remove locking when accessing driver-eventState, since it's using self-locking APIs and the pointer is immutable. --- src/test/test_driver.c | 99 -- 1 file changed, 31 insertions(+), 68 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1d54639..2ab0402 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -163,8 +163,6 @@ testDriverFree(testDriverPtr driver) VIR_FREE(driver); } -static void testObjectEventQueue(testDriverPtr driver, - virObjectEventPtr event); static void testDriverLock(testDriverPtr driver) { @@ -176,6 +174,15 @@ static void testDriverUnlock(testDriverPtr driver) virMutexUnlock(driver-lock); } +static void testObjectEventQueue(testDriverPtr driver, + virObjectEventPtr event) +{ +if (!event) +return; + +virObjectEventStateQueue(driver-eventState, event); +} + static void *testDomainObjPrivateAlloc(void) { testDomainObjPrivatePtr priv; @@ -1769,8 +1776,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, cleanup: if (dom) virObjectUnlock(dom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); virDomainDefFree(def); testDriverUnlock(privconn); return ret; @@ -1895,8 +1901,7 @@ static int testDomainDestroy(virDomainPtr domain) ret = 0; cleanup: virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -1933,11 +1938,7 @@ static int testDomainResume(virDomainPtr domain) cleanup: virDomainObjEndAPI(privdom); -if (event) { -testDriverLock(privconn); -testObjectEventQueue(privconn, event); -testDriverUnlock(privconn); -} +testObjectEventQueue(privconn, event); return ret; } @@ -1974,12 +1975,7 @@ static int testDomainSuspend(virDomainPtr domain) cleanup: virDomainObjEndAPI(privdom); - -if (event) { -testDriverLock(privconn); -testObjectEventQueue(privconn, event); -testDriverUnlock(privconn); -} +testObjectEventQueue(privconn, event); return ret; } @@ -2019,8 +2015,7 @@ static int testDomainShutdownFlags(virDomainPtr domain, ret = 0; cleanup: virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -2091,8 +2086,7 @@ static int testDomainReboot(virDomainPtr domain, ret = 0; cleanup: virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -2257,8 +2251,7 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, unlink(path); } virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -2362,8 +2355,7 @@ testDomainRestoreFlags(virConnectPtr conn, VIR_FORCE_CLOSE(fd); if (dom) virObjectUnlock(dom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -2436,8 +2428,7 @@ static int testDomainCoreDumpWithFormat(virDomainPtr domain, cleanup: VIR_FORCE_CLOSE(fd); virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -2932,8 +2923,7 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn, virDomainDefFree(oldDef); if (dom) virObjectUnlock(dom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -3073,8 +3063,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) cleanup: virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -3145,8 +3134,7 @@ static int testDomainUndefineFlags(virDomainPtr domain, cleanup: virDomainObjEndAPI(privdom); -if (event) -testObjectEventQueue(privconn, event); +testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -3579,8 +3567,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const char *xml)
[libvirt] [PATCH v2 00/15] test driver refactors
Version 2 has a few more fixes as suggested by Michal. Peter Krempa (15): test: Rename testConn to testDriver test: Drop useless forward declaration test: turn 'defaultConn' into a pointer test: Extract code to free testDriver into testDriverFree test: Extract common parts of test driver data allocation test: Drop unused attribute @path from testDriver struct test: Annotate few fields of testDriver structure test: Use atomic access to @nextDomID in struct virTestDriver test: Refactor test driver event sending test: Finalize removal of locking from driver-eventState test: Drop locked access to testDriver-domains test: Refactor test driver domain object retrieval test: Refactor testDomainSetVcpusFlags test: Refactor vcpu pinning and vcpu info retrieval test: Refactor testNodeGetCPUMap src/test/test_driver.c | 1305 +++- 1 file changed, 401 insertions(+), 904 deletions(-) -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] remote: move connection close callback to driver level
1. Introduce connect(Un)RegisterCloseCallback driver functions. 2. virConnect(Un)RegisterCloseCallback now works through driver. 3. virConnectCloseCallback is factored from virConnect but mostly stay the same. Notice however that virConnect object is not referenced in virConnectCloseCallback anymore. It is safe. Explanation. Previous version of callback object keeps reference to connection. This leads to undocumented rule that all clients must exlicitly unregister close callback before closing connection or connection will never be disposed. As callback unregistering and close event delivering are serialized thru callback object lock and unregistering zeros connection object we will never get dangling pointer on delivering. 4. callback object doesn't check callback on unregistering. The reason is that it will helps us write registering/unregistering with atomic behaviour for remote driver as it can be seen in next patch. Moreover it is not really meaningful to check callback on unregistering. 5. virNetClientSetCloseCallback call is removed from doRemoteClose as it is excessive for the same reasons as in point 3. Unregistering MUST be called and this prevents from firing event on close initiated by client. I'm not sure where callback object should be so it stays in datatype.c Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- src/datatypes.c| 112 +--- src/datatypes.h| 21 ++-- src/driver-hypervisor.h| 12 + src/libvirt-host.c | 79 ++- src/remote/remote_driver.c | 67 -- 5 files changed, 179 insertions(+), 112 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 12bcfc1..07212d2 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -34,7 +34,7 @@ VIR_LOG_INIT(datatypes); virClassPtr virConnectClass; -virClassPtr virConnectCloseCallbackDataClass; +virClassPtr virConnectCloseCallbackClass; virClassPtr virDomainClass; virClassPtr virDomainSnapshotClass; virClassPtr virInterfaceClass; @@ -47,7 +47,7 @@ virClassPtr virStorageVolClass; virClassPtr virStoragePoolClass; static void virConnectDispose(void *obj); -static void virConnectCloseCallbackDataDispose(void *obj); +static void virConnectCloseCallbackDispose(void *obj); static void virDomainDispose(void *obj); static void virDomainSnapshotDispose(void *obj); static void virInterfaceDispose(void *obj); @@ -78,7 +78,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS_COMMON(basename, virClassForObjectLockable()) DECLARE_CLASS_LOCKABLE(virConnect); -DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData); +DECLARE_CLASS_LOCKABLE(virConnectCloseCallback); DECLARE_CLASS(virDomain); DECLARE_CLASS(virDomainSnapshot); DECLARE_CLASS(virInterface); @@ -119,14 +119,7 @@ virGetConnect(void) if (!(ret = virObjectLockableNew(virConnectClass))) return NULL; -if (!(ret-closeCallback = virObjectLockableNew(virConnectCloseCallbackDataClass))) -goto error; - return ret; - - error: -virObjectUnref(ret); -return NULL; } /** @@ -147,36 +140,99 @@ virConnectDispose(void *obj) virResetError(conn-err); virURIFree(conn-uri); +} -if (conn-closeCallback) { -virObjectLock(conn-closeCallback); -conn-closeCallback-callback = NULL; -virObjectUnlock(conn-closeCallback); +virConnectCloseCallbackPtr +virGetConnectCloseCallback(void) +{ +virConnectCloseCallbackPtr ret; -virObjectUnref(conn-closeCallback); -} +if (virDataTypesInitialize() 0) +return NULL; + +if (!(ret = virObjectLockableNew(virConnectCloseCallbackClass))) +return NULL; + +return ret; } +static void +virConnectCloseCallbackClean(virConnectCloseCallbackPtr obj) +{ +if (obj-freeCallback) +obj-freeCallback(obj-opaque); + +obj-callback = NULL; +obj-freeCallback = NULL; +obj-opaque = NULL; +obj-conn = NULL; +} -/** - * virConnectCloseCallbackDataDispose: - * @obj: the close callback data to release - * - * Release resources bound to the connection close callback. - */ static void -virConnectCloseCallbackDataDispose(void *obj) +virConnectCloseCallbackDispose(void *obj ATTRIBUTE_UNUSED) +{ +// nothing really to to here +} + +int +virConnectCloseCallbackRegister(virConnectCloseCallbackPtr obj, +virConnectPtr conn, +virConnectCloseFunc cb, +void *opaque, +virFreeCallback freecb) { -virConnectCloseCallbackDataPtr cb = obj; +int ret = -1; + +virObjectLock(obj); -virObjectLock(cb); +if (obj-callback) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(A close callback is already registered)); +goto finish; +} -if (cb-freeCallback) -
[libvirt] [sandbox PATCH 02/10] Add configuration object for disk support
Add the config gobject, functions to store and load the new configuration fragments and test. This will allow creating sandboxes with attached disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2 --- libvirt-sandbox/Makefile.am | 2 + libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 +++ libvirt-sandbox/libvirt-sandbox-config.c | 293 ++ libvirt-sandbox/libvirt-sandbox-config.h | 10 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 4 + libvirt-sandbox/tests/test-config.c | 11 + 8 files changed, 677 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 6917f04..8237c50 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -55,6 +55,7 @@ SANDBOX_HEADER_FILES = \ libvirt-sandbox-main.h \ libvirt-sandbox-util.h \ libvirt-sandbox-config.h \ + libvirt-sandbox-config-disk.h \ libvirt-sandbox-config-network.h \ libvirt-sandbox-config-network-address.h \ libvirt-sandbox-config-network-filterref-parameter.h \ @@ -86,6 +87,7 @@ SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ + libvirt-sandbox-config-disk.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ libvirt-sandbox-config-network-filterref.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-config-disk.c b/libvirt-sandbox/libvirt-sandbox-config-disk.c new file mode 100644 index 000..04f1cf0 --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-config-disk.c @@ -0,0 +1,274 @@ +/* + * libvirt-sandbox-config-disk.c: libvirt sandbox configuration + * + * Copyright (C) 2015 Universitat Polit??cnica de Catalunya. + * + * 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 even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran erenyagdi...@gmail.com + */ + +#include config.h +#include string.h + +#include libvirt-sandbox/libvirt-sandbox.h + +/** + * SECTION: libvirt-sandbox-config-disk + * @short_description: Disk attachment configuration details + * @include: libvirt-sandbox/libvirt-sandbox.h + * @see_aloso: #GVirSandboxConfig + * + * Provides an object to store information about a disk attachment in the sandbox + * + */ + +#define GVIR_SANDBOX_CONFIG_DISK_GET_PRIVATE(obj) \ +(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK, GVirSandboxConfigDiskPrivate)) + + +struct _GVirSandboxConfigDiskPrivate +{ +GVirConfigDomainDiskType type; +gchar *target; +gchar *source; +GVirConfigDomainDiskFormat format; +}; + +G_DEFINE_TYPE(GVirSandboxConfigDisk, gvir_sandbox_config_disk, G_TYPE_OBJECT); + + +enum { +PROP_0, +PROP_TYPE, +PROP_TARGET, +PROP_SOURCE, +PROP_FORMAT +}; + +enum { +LAST_SIGNAL +}; + + + +static void gvir_sandbox_config_disk_get_property(GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ +GVirSandboxConfigDisk *config = GVIR_SANDBOX_CONFIG_DISK(object); +GVirSandboxConfigDiskPrivate *priv = config-priv; + +switch (prop_id) { +case PROP_TARGET: +g_value_set_string(value, priv-target); +break; +case PROP_SOURCE: +g_value_set_string(value, priv-source); +break; +case PROP_TYPE: +g_value_set_enum(value, priv-type); +break; +case PROP_FORMAT: +g_value_set_enum(value, priv-format); +break; +default: +G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); +} +} + + +static void
[libvirt] [sandbox PATCH 10/10] Common-builder: /dev/disk/by-tag/thetag to /dev/vdN
Common builder counts the disks devices and populates disks.cfg according to that.Disk devices are always come first than host-based images.In builder-machine, mounts of the host-based images will be mounted later. --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 6 +- libvirt-sandbox/libvirt-sandbox-builder.c | 73 +-- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 4148d4b..db956cf 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -283,9 +283,10 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * error); gboolean ret = FALSE; GList *mounts = gvir_sandbox_config_get_mounts(config); +GList *disks = gvir_sandbox_config_get_disks(config); GList *tmp = NULL; size_t nHostBind = 0; -size_t nHostImage = 0; +guint nVirtioDev = g_list_length(disks); if (!fos) goto cleanup; @@ -304,7 +305,7 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * fstype = 9p; options = g_strdup(trans=virtio,version=9p2000.u); } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_HOST_IMAGE(mconfig)) { -source = g_strdup_printf(/dev/vd%c, (char)('a' + nHostImage++)); +source = g_strdup_printf(/dev/vd%c, (char)('a' + nVirtioDev++)); fstype = ext4; options = g_strdup(); } else if (GVIR_SANDBOX_IS_CONFIG_MOUNT_GUEST_BIND(mconfig)) { @@ -351,6 +352,7 @@ static gboolean gvir_sandbox_builder_machine_write_mount_cfg(GVirSandboxConfig * cleanup: g_list_foreach(mounts, (GFunc)g_object_unref, NULL); g_list_free(mounts); +g_list_free(disks); if (fos) g_object_unref(fos); if (!ret) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index bcad652..d83fd9f 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -312,14 +312,75 @@ static gboolean gvir_sandbox_builder_construct_features(GVirSandboxBuilder *buil return TRUE; } +static gboolean gvir_sandbox_builder_construct_disk_cfg(GVirSandboxConfig *config, +const gchar *statedir, +GError **error) +{ -static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *builder G_GNUC_UNUSED, - GVirSandboxConfig *config G_GNUC_UNUSED, - const gchar *statedir G_GNUC_UNUSED, - GVirConfigDomain *domain G_GNUC_UNUSED, - GError **error G_GNUC_UNUSED) +guint nVirtioDev = 0; +gchar *dskfile = g_strdup_printf(%s/config/disks.cfg, statedir); +GFile *file = g_file_new_for_path(dskfile); +GFileOutputStream *fos = g_file_replace(file, +NULL, +FALSE, +G_FILE_CREATE_REPLACE_DESTINATION, +NULL, +error); +gboolean ret = FALSE; +GList *disks = gvir_sandbox_config_get_disks(config); +GList *tmp = NULL; +const gchar *target; + +if (!fos) +goto cleanup; + +tmp = disks; +while (tmp) { +GVirSandboxConfigDisk *mconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); +gchar *device = g_strdup_printf(/dev/vd%c, (char)('a' + (nVirtioDev)++)); +gchar *line; + +target = gvir_sandbox_config_disk_get_target(mconfig); + +line = g_strdup_printf(%s\t%s\n, + target, device); +g_free(device); + +if (!g_output_stream_write_all(G_OUTPUT_STREAM(fos), + line, strlen(line), + NULL, NULL, error)) { +g_free(line); +goto cleanup; +} +g_free(line); + +tmp = tmp-next; +} + +if (!g_output_stream_close(G_OUTPUT_STREAM(fos), NULL, error)) +goto cleanup; + +ret = TRUE; + cleanup: +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); +if (fos) +g_object_unref(fos); +if (!ret) +g_file_delete(file, NULL, NULL); +g_object_unref(file); +g_free(dskfile); +return ret; + +} + +static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *builder, + GVirSandboxConfig *config, +
[libvirt] [sandbox PATCH 00/10] Patches for libvirt-sandbox
Hello, These patches provide disk support for libvirt-sandbox. Implemented '--disk' parameter will be useful when integrating Docker image support for libvirt-sandbox. --Main diffs compared to previous patches. Since many hypervisors, including kvm, will not even honour requested names for disk devices we link each device under /dev/disk/by-tag e.g /dev/disk/by-tag/foobar - /dev/sda We populate disks.cfg with tag to device mapping when we build the sandbox. After that, in each init-process {Common,Qemu}, we basically read the configuration and populate the right symlinks under /dev/disk/by-tag The common functions for modifying directories are moved under Init-util. {Common,Qemu} inits are using them. Cédric Bosdonnat (2): Add gvir_sandbox_config_has_disks function qemu: use devtmpfs rather than tmpfs to auto-populate /dev Eren Yagdiran (8): Add an utility function for guessing filetype from file extension Add configuration object for disk support Add disk parameter to virt-sandbox Add disk support to the container builder Add disk support to machine builder Init-util : Common directory functions for init-common and init-qemu Common-init: Building symlink from disks.cfg Common-builder: /dev/disk/by-tag/thetag to /dev/vdN bin/virt-sandbox.c | 37 +++ libvirt-sandbox/Makefile.am| 7 +- .../libvirt-sandbox-builder-container.c| 36 ++- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 44 ++- libvirt-sandbox/libvirt-sandbox-builder.c | 73 - libvirt-sandbox/libvirt-sandbox-config-disk.c | 274 +++ libvirt-sandbox/libvirt-sandbox-config-disk.h | 82 ++ libvirt-sandbox/libvirt-sandbox-config.c | 300 + libvirt-sandbox/libvirt-sandbox-config.h | 11 + libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +++- libvirt-sandbox/libvirt-sandbox-init-qemu.c| 151 ++- libvirt-sandbox/libvirt-sandbox-init-util.c| 58 libvirt-sandbox/libvirt-sandbox-init-util.h| 41 +++ libvirt-sandbox/libvirt-sandbox-util.c | 72 + libvirt-sandbox/libvirt-sandbox-util.h | 5 + libvirt-sandbox/libvirt-sandbox.h | 1 + libvirt-sandbox/libvirt-sandbox.sym| 5 + libvirt-sandbox/tests/test-config.c| 11 + po/POTFILES.in | 1 + 19 files changed, insertions(+), 149 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 09/10] Common-init: Building symlink from disks.cfg
Similar to the existing mounts.cfg, the mapping between the device and the tag is passed by a new disks.cfg file. Common-init reads disks.cfg and maps the tags to corresponding devices --- libvirt-sandbox/libvirt-sandbox-init-common.c | 51 +-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-common.c b/libvirt-sandbox/libvirt-sandbox-init-common.c index 68f96ba..f8b2ea5 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-common.c +++ b/libvirt-sandbox/libvirt-sandbox-init-common.c @@ -46,6 +46,7 @@ #include grp.h #include libvirt-sandbox-rpcpacket.h +#include libvirt-sandbox-init-util.h static gboolean debug = FALSE; static gboolean verbose = FALSE; @@ -60,6 +61,51 @@ static void sig_child(int sig ATTR_UNUSED) abort(); } +static gboolean setup_disk_tags(void) { +FILE *fp; +gboolean ret = FALSE; +static char line[1024]; +if (debug) +fprintf(stderr, libvirt-sandbox-init-common: %s: populate /dev/disk/by-tag/\n, + __func__); +fp = fopen(SANDBOXCONFIGDIR /disks.cfg, r); +if (fp == NULL) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot open SANDBOXCONFIGDIR /disks.cfg: %s\n, +__func__, strerror(errno)); + +goto cleanup; +} +gvir_sandbox_init_util_mkdir(/dev/disk/by-tag, 0755, debug == TRUE ? 1 : 0); +while (fgets(line, sizeof line, fp)) { +char path[1024]; +char *tag = line; +char *device = strchr(tag, '\t'); +*device = '\0'; +device++; +char *tmp = strchr(device, '\n'); +*tmp = '\0'; + +if (sprintf(path, /dev/disk/by-tag/%s, tag) 0) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot compute disk path: %s\n, +__func__, strerror(errno)); +goto cleanup; +} + +if (debug) +fprintf(stderr, libvirt-sandbox-init-common: %s: %s - %s\n, +__func__, path, device); + +if (symlink(device, path) 0) { +fprintf(stderr, libvirt-sandbox-init-common: %s: cannot create symlink %s - %s: %s\n, +__func__, path, device, strerror(errno)); +goto cleanup; +} +} +ret = TRUE; + cleanup: +fclose(fp); +return ret; +} static int start_shell(void) @@ -258,8 +304,6 @@ static gboolean setup_network_device(GVirSandboxConfigNetwork *config, return ret; } - - static gboolean setup_network(GVirSandboxConfig *config, GError **error) { int i = 0; @@ -1215,6 +1259,9 @@ int main(int argc, char **argv) { start_shell() 0) exit(EXIT_FAILURE); +if (!setup_disk_tags()) +exit(EXIT_FAILURE); + if (!setup_network(config, error)) goto error; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 06/10] Add gvir_sandbox_config_has_disks function
From: Cédric Bosdonnat cbosdon...@suse.com Add helper function to check if a config contains disk devices. --- libvirt-sandbox/libvirt-sandbox-config.c | 7 +++ libvirt-sandbox/libvirt-sandbox-config.h | 1 + libvirt-sandbox/libvirt-sandbox.sym | 1 + 3 files changed, 9 insertions(+) diff --git a/libvirt-sandbox/libvirt-sandbox-config.c b/libvirt-sandbox/libvirt-sandbox-config.c index 3342706..f800cf9 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.c +++ b/libvirt-sandbox/libvirt-sandbox-config.c @@ -1305,6 +1305,13 @@ gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, } +gboolean gvir_sandbox_config_has_disks(GVirSandboxConfig *config) +{ +GVirSandboxConfigPrivate *priv = config-priv; +return priv-disks != NULL; +} + + /** * gvir_sandbox_config_add_mount: * @config: (transfer none): the sandbox config diff --git a/libvirt-sandbox/libvirt-sandbox-config.h b/libvirt-sandbox/libvirt-sandbox-config.h index deaea68..ebbebf2 100644 --- a/libvirt-sandbox/libvirt-sandbox-config.h +++ b/libvirt-sandbox/libvirt-sandbox-config.h @@ -131,6 +131,7 @@ gboolean gvir_sandbox_config_add_disk_strv(GVirSandboxConfig *config, gboolean gvir_sandbox_config_add_disk_opts(GVirSandboxConfig *config, const char *disk, GError **error); +gboolean gvir_sandbox_config_has_disks(GVirSandboxConfig *config); void gvir_sandbox_config_add_mount(GVirSandboxConfig *config, GVirSandboxConfigMount *mnt); diff --git a/libvirt-sandbox/libvirt-sandbox.sym b/libvirt-sandbox/libvirt-sandbox.sym index bb717ed..e5f8660 100644 --- a/libvirt-sandbox/libvirt-sandbox.sym +++ b/libvirt-sandbox/libvirt-sandbox.sym @@ -217,4 +217,5 @@ LIBVIRT_SANDBOX_0.5.2 { gvir_sandbox_config_add_disk_strv; gvir_sandbox_config_add_disk_opts; gvir_sandbox_config_disk_get_type; +gvir_sandbox_config_has_disks; } LIBVIRT_SANDBOX_0.2.1; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/15] test: turn 'defaultConn' into a pointer
--- src/test/test_driver.c | 46 -- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 8d2bb5b..ff383c6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -117,7 +117,7 @@ struct _testDriver { typedef struct _testDriver testDriver; typedef testDriver *testDriverPtr; -static testDriver defaultConn; +static testDriverPtr defaultConn; static int defaultConnections; static virMutex defaultLock = VIR_MUTEX_INITIALIZER; @@ -691,7 +691,7 @@ static int testOpenDefault(virConnectPtr conn) { int u; -testDriverPtr privconn = defaultConn; +testDriverPtr privconn = NULL; virDomainDefPtr domdef = NULL; virDomainObjPtr domobj = NULL; virNetworkDefPtr netdef = NULL; @@ -705,17 +705,18 @@ testOpenDefault(virConnectPtr conn) virMutexLock(defaultLock); if (defaultConnections++) { -conn-privateData = defaultConn; +conn-privateData = defaultConn; virMutexUnlock(defaultLock); return VIR_DRV_OPEN_SUCCESS; } +if (VIR_ALLOC(privconn) 0) +goto error; + if (virMutexInit(privconn-lock) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot initialize mutex)); -defaultConnections--; -virMutexUnlock(defaultLock); -return VIR_DRV_OPEN_ERROR; +goto error; } conn-privateData = privconn; @@ -822,19 +823,23 @@ testOpenDefault(virConnectPtr conn) } virNodeDeviceObjUnlock(nodeobj); +defaultConn = privconn; + virMutexUnlock(defaultLock); return VIR_DRV_OPEN_SUCCESS; error: -virObjectUnref(privconn-domains); -virObjectUnref(privconn-networks); -virInterfaceObjListFree(privconn-ifaces); -virStoragePoolObjListFree(privconn-pools); -virNodeDeviceObjListFree(privconn-devs); -virObjectUnref(privconn-caps); -virObjectEventStateFree(privconn-eventState); -virMutexDestroy(privconn-lock); +if (privconn) { +virObjectUnref(privconn-domains); +virObjectUnref(privconn-networks); +virInterfaceObjListFree(privconn-ifaces); +virStoragePoolObjListFree(privconn-pools); +virNodeDeviceObjListFree(privconn-devs); +virObjectUnref(privconn-caps); +virObjectEventStateFree(privconn-eventState); +virMutexDestroy(privconn-lock); +} conn-privateData = NULL; virDomainDefFree(domdef); defaultConnections--; @@ -1573,8 +1578,10 @@ static virDrvOpenStatus testConnectOpen(virConnectPtr conn, static int testConnectClose(virConnectPtr conn) { testDriverPtr privconn = conn-privateData; +bool dflt = false; -if (privconn == defaultConn) { +if (privconn == defaultConn) { +dflt = true; virMutexLock(defaultLock); if (--defaultConnections) { virMutexUnlock(defaultLock); @@ -1596,10 +1603,13 @@ static int testConnectClose(virConnectPtr conn) testDriverUnlock(privconn); virMutexDestroy(privconn-lock); -if (privconn == defaultConn) +VIR_FREE(privconn); + +if (dflt) { +defaultConn = NULL; virMutexUnlock(defaultLock); -else -VIR_FREE(privconn); +} + conn-privateData = NULL; return 0; } -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 15/15] test: Refactor testNodeGetCPUMap
Drop locking of the driver since it is not accessed and simplify the code flow. --- src/test/test_driver.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ed67dca..25de641 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5611,31 +5611,23 @@ static int testConnectListAllDomains(virConnectPtr conn, } static int -testNodeGetCPUMap(virConnectPtr conn, +testNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned char **cpumap, unsigned int *online, unsigned int flags) { -testDriverPtr privconn = conn-privateData; -int ret = -1; - virCheckFlags(0, -1); -testDriverLock(privconn); if (cpumap) { if (VIR_ALLOC_N(*cpumap, 1) 0) -goto cleanup; +return -1; *cpumap[0] = 0x15; } if (online) *online = 3; -ret = 8; - - cleanup: -testDriverUnlock(privconn); -return ret; +return 8; } static char * -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Support for new watchdog model diag288 and action inject-nmi
On Wed, Jun 24, 2015 at 11:28:40AM +0200, Boris Fiuczynski wrote: This patch set provides support for the new watchdog model diag288 including the new watchdog action inject-nmi. v2: Added tests for action and model. Boris Fiuczynski (4): Support for a new watchdog action inject-nmi Test for the new watchdog action inject-nmi Support for the new watchdog model diag288 Test for the new watchdog model diag288 ACK series, now pushed. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 14/15] test: Refactor vcpu pinning and vcpu info retrieval
Drop internal data structures and use the proper fields in virDomainDef. This allows to greatly simplify the code and allows to remove the private data structure that was holding just redundant data. This patch also fixes the bogous output where we'd report that a fresh VM without vCPU pinning would not run on all vcpus. --- src/test/test_driver.c | 224 +++-- 1 file changed, 49 insertions(+), 175 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ae332ef..ed67dca 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -65,14 +65,6 @@ VIR_LOG_INIT(test.test_driver); -/* Driver specific info to carry with a domain */ -struct _testDomainObjPrivate { -virVcpuInfoPtr vcpu_infos; - -unsigned char *cpumaps; -}; -typedef struct _testDomainObjPrivate testDomainObjPrivate; -typedef struct _testDomainObjPrivate *testDomainObjPrivatePtr; #define MAX_CPUS 128 @@ -183,25 +175,6 @@ static void testObjectEventQueue(testDriverPtr driver, virObjectEventStateQueue(driver-eventState, event); } -static void *testDomainObjPrivateAlloc(void) -{ -testDomainObjPrivatePtr priv; - -if (VIR_ALLOC(priv) 0) -return NULL; - -return priv; -} - -static void testDomainObjPrivateFree(void *data) -{ -testDomainObjPrivatePtr priv = data; - -VIR_FREE(priv-vcpu_infos); -VIR_FREE(priv-cpumaps); -VIR_FREE(priv); -} - #define TEST_NAMESPACE_HREF http://libvirt.org/schemas/domain/test/1.0; typedef struct _testDomainNamespaceDef testDomainNamespaceDef; @@ -401,11 +374,6 @@ testBuildCapabilities(virConnectPtr conn) static testDriverPtr testDriverNew(void) { -virDomainXMLPrivateDataCallbacks priv = { -.alloc = testDomainObjPrivateAlloc, -.free = testDomainObjPrivateFree -}; - virDomainXMLNamespace ns = { .parse = testDomainDefNamespaceParse, .free = testDomainDefNamespaceFree, @@ -421,7 +389,7 @@ testDriverNew(void) goto error; } -if (!(ret-xmlopt = virDomainXMLOptionNew(NULL, priv, ns)) || +if (!(ret-xmlopt = virDomainXMLOptionNew(NULL, NULL, ns)) || !(ret-eventState = virObjectEventStateNew()) || !(ret-domains = virDomainObjListNew()) || !(ret-networks = virNetworkObjListNew())) @@ -601,95 +569,6 @@ testDomainGenerateIfnames(virDomainDefPtr domdef) return 0; } -/* Helper to update info for a single VCPU */ -static int -testDomainUpdateVCPU(virDomainObjPtr dom, - int vcpu, - int maplen, - int maxcpu) -{ -testDomainObjPrivatePtr privdata = dom-privateData; -virVcpuInfoPtr info = privdata-vcpu_infos[vcpu]; -unsigned char *cpumap = VIR_GET_CPUMAP(privdata-cpumaps, maplen, vcpu); -size_t j; -bool cpu; - -memset(info, 0, sizeof(virVcpuInfo)); -memset(cpumap, 0, maplen); - -info-number= vcpu; -info-state = VIR_VCPU_RUNNING; -info-cpuTime = 500; -info-cpu = 0; - -if (dom-def-cpumask) { -for (j = 0; j maxcpu j VIR_DOMAIN_CPUMASK_LEN; ++j) { -if (virBitmapGetBit(dom-def-cpumask, j, cpu) 0) -return -1; -if (cpu) { -VIR_USE_CPU(cpumap, j); -info-cpu = j; -} -} -} else { -for (j = 0; j maxcpu; ++j) { -if ((j % 3) == 0) { -/* Mark of every third CPU as usable */ -VIR_USE_CPU(cpumap, j); -info-cpu = j; -} -} -} - -return 0; -} - -/* - * Update domain VCPU amount and info - * - * @conn: virConnectPtr - * @dom : domain needing updates - * @nvcpus: New amount of vcpus for the domain - * @clear_all: If true, rebuild info for ALL vcpus, not just newly added vcpus - */ -static int -testDomainUpdateVCPUs(testDriverPtr privconn, - virDomainObjPtr dom, - int nvcpus, - unsigned int clear_all) -{ -testDomainObjPrivatePtr privdata = dom-privateData; -size_t i; -int ret = -1; -int cpumaplen, maxcpu; - -maxcpu = VIR_NODEINFO_MAXCPUS(privconn-nodeInfo); -cpumaplen = VIR_CPU_MAPLEN(maxcpu); - -if (VIR_REALLOC_N(privdata-vcpu_infos, nvcpus) 0) -goto cleanup; - -if (VIR_REALLOC_N(privdata-cpumaps, nvcpus * cpumaplen) 0) -goto cleanup; - -/* Set running VCPU and cpumap state */ -if (clear_all) { -for (i = 0; i nvcpus; ++i) -if (testDomainUpdateVCPU(dom, i, cpumaplen, maxcpu) 0) -goto cleanup; - -} else if (nvcpus dom-def-vcpus) { -/* VCPU amount has grown, populate info for the new vcpus */ -for (i = dom-def-vcpus; i nvcpus; ++i) -if (testDomainUpdateVCPU(dom, i, cpumaplen, maxcpu) 0) -goto cleanup; -} - -dom-def-vcpus = nvcpus; -ret = 0; - cleanup: -return ret; -} static void
[libvirt] [PATCH v2 12/15] test: Refactor test driver domain object retrieval
Reuse testDomObjFromDomain testDomObjFromDomain to retrieve domain objects in the rest of the test driver instead of open-coding it in every API. Now that testDomObjFromDomain does not lock the driver it can be reused also in places where the driver is already locked. --- src/test/test_driver.c | 390 ++--- 1 file changed, 77 insertions(+), 313 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c36e0f8..2c2f009 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1684,43 +1684,28 @@ static int testConnectNumOfDomains(virConnectPtr conn) static int testDomainIsActive(virDomainPtr dom) { -testDriverPtr privconn = dom-conn-privateData; virDomainObjPtr obj; -int ret = -1; +int ret; -testDriverLock(privconn); -obj = virDomainObjListFindByUUID(privconn-domains, dom-uuid); -testDriverUnlock(privconn); -if (!obj) { -virReportError(VIR_ERR_NO_DOMAIN, NULL); -goto cleanup; -} -ret = virDomainObjIsActive(obj); +if (!(obj = testDomObjFromDomain(dom))) +return -1; - cleanup: -if (obj) -virObjectUnlock(obj); +ret = virDomainObjIsActive(obj); +virDomainObjEndAPI(obj); return ret; } static int testDomainIsPersistent(virDomainPtr dom) { -testDriverPtr privconn = dom-conn-privateData; virDomainObjPtr obj; -int ret = -1; +int ret; + +if (!(obj = testDomObjFromDomain(dom))) +return -1; -testDriverLock(privconn); -obj = virDomainObjListFindByUUID(privconn-domains, dom-uuid); -testDriverUnlock(privconn); -if (!obj) { -virReportError(VIR_ERR_NO_DOMAIN, NULL); -goto cleanup; -} ret = obj-persistent; - cleanup: -if (obj) -virObjectUnlock(obj); +virDomainObjEndAPI(obj); return ret; } @@ -1863,13 +1848,9 @@ static int testDomainDestroy(virDomainPtr domain) virObjectEventPtr event = NULL; int ret = -1; -privdom = virDomainObjListFindByName(privconn-domains, - domain-name); -if (privdom == NULL) { -virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); +if (!(privdom = testDomObjFromDomain(domain))) goto cleanup; -} testDomainShutdownState(domain, privdom, VIR_DOMAIN_SHUTOFF_DESTROYED); event = virDomainEventLifecycleNewFromObj(privdom, @@ -1893,15 +1874,8 @@ static int testDomainResume(virDomainPtr domain) virObjectEventPtr event = NULL; int ret = -1; -testDriverLock(privconn); -privdom = virDomainObjListFindByName(privconn-domains, - domain-name); -testDriverUnlock(privconn); - -if (privdom == NULL) { -virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); -goto cleanup; -} +if (!(privdom = testDomObjFromDomain(domain))) +return -1; if (virDomainObjGetState(privdom, NULL) != VIR_DOMAIN_PAUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, _(domain '%s' not paused), @@ -1930,15 +1904,8 @@ static int testDomainSuspend(virDomainPtr domain) int ret = -1; int state; -testDriverLock(privconn); -privdom = virDomainObjListFindByName(privconn-domains, - domain-name); -testDriverUnlock(privconn); - -if (privdom == NULL) { -virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); -goto cleanup; -} +if (!(privdom = testDomObjFromDomain(domain))) +return -1; state = virDomainObjGetState(privdom, NULL); if (state == VIR_DOMAIN_SHUTOFF || state == VIR_DOMAIN_PAUSED) { @@ -1969,13 +1936,9 @@ static int testDomainShutdownFlags(virDomainPtr domain, virCheckFlags(0, -1); -privdom = virDomainObjListFindByName(privconn-domains, - domain-name); -if (privdom == NULL) { -virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); +if (!(privdom = testDomObjFromDomain(domain))) goto cleanup; -} if (virDomainObjGetState(privdom, NULL) == VIR_DOMAIN_SHUTOFF) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2012,13 +1975,9 @@ static int testDomainReboot(virDomainPtr domain, virObjectEventPtr event = NULL; int ret = -1; -privdom = virDomainObjListFindByName(privconn-domains, - domain-name); -if (privdom == NULL) { -virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); +if (!(privdom = testDomObjFromDomain(domain))) goto cleanup; -} virDomainObjSetState(privdom, VIR_DOMAIN_SHUTDOWN, VIR_DOMAIN_SHUTDOWN_USER); @@ -2070,20 +2029,12 @@ static int testDomainReboot(virDomainPtr domain, static int testDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { -testDriverPtr privconn = domain-conn-privateData; struct timeval tv; virDomainObjPtr
[libvirt] [PATCH 2/3] daemon: relay connection close event related functions
Add conneciton close subscription/unsubscription and event rpc. Now remote driver firing connection close event in 2 cases. 1. connection to daemon closed, as previously 2. as a relay of connection close event from wrapped driver As it commented out in remoteDispatchConnectCloseCallbackRegister we impose some multi-thread requirements on drivers implementations. This is the same approach as in for example remoteDispatchConnectDomainEventRegister. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com --- daemon/libvirtd.h|1 + daemon/remote.c | 87 ++ src/remote/remote_driver.c | 50 +-- src/remote/remote_protocol.x | 24 +++- src/remote_protocol-structs |6 +++ 5 files changed, 162 insertions(+), 6 deletions(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 8c1a904..5b2d2ca 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -60,6 +60,7 @@ struct daemonClientPrivate { size_t nnetworkEventCallbacks; daemonClientEventCallbackPtr *qemuEventCallbacks; size_t nqemuEventCallbacks; +bool closeRegistered; # if WITH_SASL virNetSASLSessionPtr sasl; diff --git a/daemon/remote.c b/daemon/remote.c index e9e2dca..e0cc008 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1189,6 +1189,20 @@ remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, VIR_FREE(details_p); } +static +void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int reason, void *opaque) +{ +virNetServerClientPtr client = opaque; + +VIR_DEBUG(Relaying connection closed event, reason %d, reason); + +remote_connect_event_connection_closed_msg msg = { reason }; +remoteDispatchObjectEventSend(client, remoteProgram, + REMOTE_PROC_CONNECT_CLOSE, + (xdrproc_t)xdr_remote_connect_event_connection_closed_msg, + msg); +} + /* * You must hold lock for at least the client * We don't free stuff here, merely disconnect the client's @@ -1251,6 +1265,13 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv-qemuEventCallbacks); +if (priv-closeRegistered) { +if (virConnectUnregisterCloseCallback( +priv-conn, +remoteRelayConnectionClosedEvent) 0) +VIR_WARN(unexpected close callback event deregister failure); +} + virConnectClose(priv-conn); virIdentitySetCurrent(NULL); @@ -3489,6 +3510,72 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server ATTRIBUTE_UNUSED, return rv; } +static int +remoteDispatchConnectCloseCallbackRegister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr) +{ +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +virMutexLock(priv-lock); + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +// NetClient is passed to driver but not referenced. +// This imposes next requirements on drivers implementation. +// Driver must serialize unregistering and event delivering operations. +// Thus as we unregister callback before unreferencing NetClient +// remoteRelayConnectionClosedEvent is safe to use NetClient. +if (virConnectRegisterCloseCallback(priv-conn, remoteRelayConnectionClosedEvent, client, NULL) 0) +goto cleanup; + +priv-closeRegistered = true; + +rv = 0; + + cleanup: +virMutexUnlock(priv-lock); +if (rv 0) +virNetMessageSaveError(rerr); +return rv; +} + +static int +remoteDispatchConnectCloseCallbackUnregister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr) +{ +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +virMutexLock(priv-lock); + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (virConnectUnregisterCloseCallback(priv-conn, remoteRelayConnectionClosedEvent) 0) +goto cleanup; + +priv-closeRegistered = false; + +rv = 0; + + cleanup: +virMutexUnlock(priv-lock); +if (rv 0) +virNetMessageSaveError(rerr); +return rv; +} /*** * Register / deregister events diff --git a/src/remote/remote_driver.c
Re: [libvirt] [PATCH] qemu: check hostdev address type
On 06/17/2015 12:00 PM, Ján Tomko wrote: For USB and SCSI hostdevs, we passed the invalid address to QEMU. Report an error earlier. PCI hostdevs check the address type when parsing the XML. https://bugzilla.redhat.com/show_bug.cgi?id=1225339 --- src/qemu/qemu_command.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) So does this more or less obviate the need for my change to domain_conf.c here: http://www.redhat.com/archives/libvir-list/2015-June/msg01105.html I know it's always a decision whether to fail at parse or run time and we usually defer to run time; however, in this case it seems more reasonable to check at parse instead of run since it's a bogus XML address provided. Understand that the 'scsi' domain could disappear in the bz example provided, but would that device even be usable anyway? I suppose for the pci type it's fortunate that virDomainDeviceDriveAddressPtr and virDevicePCIAddressPtr are similar enough to within their union to not be cause an issue. I'm not opposed to this change as it's technically correct, just want to understand why not check sooner. John diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3886b4f..a4853ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10572,7 +10572,12 @@ qemuBuildCommandLine(virConnectPtr conn, /* USB */ if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - +if (hostdev-info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +hostdev-info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(USB host devices must use 'usb' address type)); +goto error; +} if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { virCommandAddArg(cmd, -device); if (!(devstr = qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps))) @@ -10644,6 +10649,12 @@ qemuBuildCommandLine(virConnectPtr conn, /* SCSI */ if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { +if (hostdev-info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +hostdev-info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(SCSI host devices must use 'drive' address type)); +goto error; +} if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix wrong remove guest cfg if migrate fail
If we get fail in qemuMigrationPrepareAny, we forget check if the vm is persistent then always call qemuDomainRemoveInactive to clean the inactive settings. Add a check to avoid this. This issue was introduce in commit 540c339. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 47d49cd..a57a177 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3432,7 +3432,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_FREE(priv-origname); virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort); priv-nbdPort = 0; -qemuDomainRemoveInactive(driver, vm); +if (!vm-persistent) +qemuDomainRemoveInactive(driver, vm); } virDomainObjEndAPI(vm); if (event) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:conf: introduce a function to delete vcpu sched
On 06/24/2015 08:51 PM, Peter Krempa wrote: On Wed, Jun 24, 2015 at 16:44:22 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1235180 We have API allow vpu to be deleted, but an vcpu may be included in some domain vcpu sched, so add a new API to allow removing an iothread from some entry. Split the virDomainIOThreadSchedDelId to reuse some code. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 48 ++-- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 6 -- 4 files changed, 40 insertions(+), 16 deletions(-) I'm going to refactor the data structures that are storing the thread scheduler data which will inherently fix this issue so even if we apply this patch it will be basically reverted very soon. Yes, i see your comment in this bug (unfortunately this is after i send this patch :) ), i agree no need apply this patch if you will fix that issue together during refactor. Thanks for your quick review. Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Additional SCSI generated device address checks
On 06/22/2015 05:05 PM, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1210587 These patches will resolve a couple issues with generation of the address type='drive' .../ for a SCSI disk and hostdev. The disk generation algorithm 'assumes' that when presented with target dev='sda'.../ that it can use controller=0 and unit=0 since sda would conceivably be the first device; however, a hostdev could attempt to assign itself to that address and it doesn't have a target device name, so it bypasses the virDomainDiskDefDstDuplicates checks that would normally 'catch' two disk's attempting to use the same name. Likewise, if a hostdev occupies an address and we attempt to hotplug a disk without providing an address, the address generation could attempt to place the disk on the already existing host device. John Ferlan (5): conf: Enforce SCSI hostdev address type conf: Add 'bus' and 'target' to SCSI address conflict checks conf: Add SCSI hostdev check for disk drive address already in use conf: Refactor virDomainDiskDefParseXML to pass vmdef conf: Check for hostdev conflicts when assign default disk address docs/formatdomain.html.in | 4 +- src/conf/domain_conf.c| 115 -- src/conf/domain_conf.h| 3 +- src/qemu/qemu_command.c | 4 +- src/vmx/vmx.c | 22 + src/vmx/vmx.h | 3 +- 6 files changed, 101 insertions(+), 50 deletions(-) For what it's worth, other than the one doc change suggestion, the rest looks (and works) fine to me. Reviewed-by: Eric Farman far...@linux.vnet.ibm.com - Eric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] conf: Enforce SCSI hostdev address type
On 06/22/2015 05:05 PM, John Ferlan wrote: If a SCSI subsystem hostdev element is provided with an address, then enforce that the address type is 'drive'. If not provided, a 'drive' element was created by virDomainHostdevAssignAddress which uses VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE. Signed-off-by: John Ferlan jfer...@redhat.com --- docs/formatdomain.html.in | 4 ++-- src/conf/domain_conf.c| 6 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 95d8c45..0475527 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3343,8 +3343,8 @@ (starting with 0x) or octal (starting with 0) form. For PCI devices the element carries 4 attributes allowing to designate the device as can be found with the codelspci/code or - with codevirsh - nodedev-list/code. a href=#elementsAddressSee above/a for + with codevirsh nodedev-list/code. For SCSI devices a 'drive' + address type is used. a href=#elementsAddressSee above/a for s/is/must be/ ? more details on the address element./dd dtcodedriver/code/dt dd diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e592adf..ce5093d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11752,6 +11752,12 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, virReportError(VIR_ERR_XML_ERROR, %s, _(SCSI host devices must have address specified)); goto error; +} else if (def-info-type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(SCSI host device must use 'drive' + address type)); +goto error; } if (virXPathBoolean(boolean(./readonly), ctxt)) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 12/15] test: Refactor test driver domain object retrieval
On Wed, Jun 24, 2015 at 04:11:54PM +0200, Peter Krempa wrote: Reuse testDomObjFromDomain testDomObjFromDomain to retrieve domain objects in the rest of the test driver instead of open-coding it in every API. Now that testDomObjFromDomain does not lock the driver it can be reused also in places where the driver is already locked. The commit message is confusing, please fix it. Pavel --- src/test/test_driver.c | 390 ++--- 1 file changed, 77 insertions(+), 313 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 03/10] Add disk parameter to virt-sandbox
Allow users to add disk images to their sandbox. Only disk images are supported so far, but the parameter is intentionally designed for future changes. --- bin/virt-sandbox.c | 37 + 1 file changed, 37 insertions(+) diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c index 1ab2f8a..ac56346 100644 --- a/bin/virt-sandbox.c +++ b/bin/virt-sandbox.c @@ -63,6 +63,7 @@ int main(int argc, char **argv) { GMainLoop *loop = NULL; GError *error = NULL; gchar *name = NULL; +gchar **disks = NULL; gchar **mounts = NULL; gchar **includes = NULL; gchar *includefile = NULL; @@ -92,6 +93,8 @@ int main(int argc, char **argv) { N_(name of the sandbox), NAME }, { root, 'r', 0, G_OPTION_ARG_STRING, root, N_(root directory of the sandbox), DIR }, +{ disk, ' ', 0, G_OPTION_ARG_STRING_ARRAY, disks, + N_(add a disk in the guest), TYPE:TARGET=SOURCE,FORMAT=FORMAT }, { mount, 'm', 0, G_OPTION_ARG_STRING_ARRAY, mounts, N_(mount a filesystem in the guest), TYPE:TARGET=SOURCE }, { include, 'i', 0, G_OPTION_ARG_STRING_ARRAY, includes, @@ -182,6 +185,13 @@ int main(int argc, char **argv) { gvir_sandbox_config_set_username(cfg, root); } +if (disks +!gvir_sandbox_config_add_disk_strv(cfg, disks, error)) { +g_printerr(_(Unable to parse disks: %s\n), + error error-message ? error-message : _(Unknown failure)); +goto cleanup; +} + if (mounts !gvir_sandbox_config_add_mount_strv(cfg, mounts, error)) { g_printerr(_(Unable to parse mounts: %s\n), @@ -319,6 +329,33 @@ inheriting the host's root filesystem. NB. CDIR must contain a matching install of the libvirt-sandbox package. This restriction may be lifted in a future version. +=item B--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT + +Sets up a disk inside the sandbox by using BSOURCE with target device BTARGET +and type BTYPE and format BFORMAT. Example: file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2 +Format is an optional parameter. + +=over 4 + +=item BTYPE + +Type parameter can be set to file. + +=item BTARGET + +Target parameter can be set to hda or any other libvirt supported target disk + +=item BSOURCE + +Source parameter needs to point a file which must be a one of the valid domain disk formats supported by qemu. + +=item BFORMAT + +Format parameter must be set to the same disk format as the file passed on source parameter. +This parameter is optional and the format can be guessed from the image extension + +=back + =item B-m TYPE:DST=SRC, B--mount TYPE:DST=SRC Sets up a mount inside the sandbox at BDST backed by BSRC. The -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 07/10] qemu: use devtmpfs rather than tmpfs to auto-populate /dev
From: Cédric Bosdonnat cbosdon...@suse.com When using devtmpfs we don't need to care about the device nodes creation: it's less risk to forget some. It also eases the creation of the devices in the init-qemu. --- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 94 + 1 file changed, 1 insertion(+), 93 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 750c9de..33cebed 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -53,7 +53,6 @@ static int has_command_arg(const char *name, static int debug = 0; static char line[1024]; -static char line2[1024]; static void exit_poweroff(void) __attribute__((noreturn)); @@ -173,50 +172,6 @@ mount_9pfs(const char *src, const char *dst, int mode, int readonly) } } -static int virtioblk_major = 0; -static void -create_virtioblk_device(const char *dev) -{ -int minor; - -if (virtioblk_major == 0) { -FILE *fp = fopen(/proc/devices, r); -if (!fp) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot read /proc/devices: %s\n, -__func__, strerror(errno)); -exit_poweroff(); -} -while (fgets(line2, sizeof line2, fp)) { -if (strstr(line2, virtblk)) { -char *end; -long l = strtol(line2, end, 10); -if (line2 == end) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot extract device major from '%s'\n, -__func__, line2); -fclose(fp); -exit_poweroff(); -} -virtioblk_major = l; -break; -} -} -fclose(fp); - -if (virtioblk_major == 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot find virtioblk device major in /proc/devices\n, -__func__); -exit_poweroff(); -} -} - -minor = (dev[strlen(dev)-1] - 'a') * 16; - -if (mknod(dev, S_IFBLK |0700, makedev(virtioblk_major, minor)) 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make dev '%s' '%llu': %s\n, -__func__, dev, makedev(virtioblk_major, minor), strerror(errno)); -exit_poweroff(); -} -} int main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) @@ -283,59 +238,14 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) } /* Main special filesystems */ -mount_other(/dev, tmpfs, 0755); +mount_other(/dev, devtmpfs, 0755); mount_other_opts(/dev/pts, devpts, gid=5,mode=620,ptmxmode=000, 0755); mount_other(/sys, sysfs, 0755); mount_other(/proc, proc, 0755); //mount_other(/selinux, selinuxfs, 0755); mount_other(/dev/shm, tmpfs, 01777); -if (mkdir(/dev/input, 0777) 0) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make directory /dev/input: %s\n, -__func__, strerror(errno)); -exit_poweroff(); -} - -#define MKNOD(file, mode, dev) \ -do {\ -if (mknod(file, mode, dev) 0) { \ -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make dev %s %llu: %s\n, \ -__func__, file, (unsigned long long)dev, strerror(errno)); \ -exit_poweroff();\ -} \ -} while (0) - -umask(); -MKNOD(/dev/null, S_IFCHR |0666, makedev(1, 3)); -MKNOD(/dev/zero, S_IFCHR |0666, makedev(1, 5)); -MKNOD(/dev/full, S_IFCHR |0666, makedev(1, 7)); -MKNOD(/dev/random, S_IFCHR |0666, makedev(1, 8)); -MKNOD(/dev/urandom, S_IFCHR |0666, makedev(1, 9)); -MKNOD(/dev/console, S_IFCHR |0700, makedev(5, 1)); -MKNOD(/dev/tty, S_IFCHR |0700, makedev(5, 0)); -MKNOD(/dev/tty0, S_IFCHR |0700, makedev(4, 0)); -MKNOD(/dev/tty1, S_IFCHR |0700, makedev(4, 1)); -MKNOD(/dev/tty2, S_IFCHR |0700, makedev(4, 2)); -MKNOD(/dev/ttyS0, S_IFCHR |0700, makedev(4, 64)); -MKNOD(/dev/ttyS1, S_IFCHR |0700, makedev(4, 65)); -MKNOD(/dev/ttyS2, S_IFCHR |0700, makedev(4, 66)); -MKNOD(/dev/ttyS3, S_IFCHR |0700, makedev(4, 67)); -MKNOD(/dev/hvc0, S_IFCHR |0700, makedev(229, 0)); -MKNOD(/dev/hvc1, S_IFCHR |0700, makedev(229, 1)); -MKNOD(/dev/hvc2, S_IFCHR |0700, makedev(229, 2)); -MKNOD(/dev/fb, S_IFCHR |0700, makedev(29, 0)); -MKNOD(/dev/fb0, S_IFCHR |0700, makedev(29, 0)); -MKNOD(/dev/mem, S_IFCHR |0600, makedev(1, 1)); -MKNOD(/dev/rtc, S_IFCHR |0700, makedev(254, 0)); -MKNOD(/dev/rtc0, S_IFCHR |0700, makedev(254, 0)); -MKNOD(/dev/ptmx, S_IFCHR |0777, makedev(5, 2)); -MKNOD(/dev/input/event0, S_IFCHR |0700,
[libvirt] [sandbox PATCH 01/10] Add an utility function for guessing filetype from file extension
Consider the file name extension as the image type, except for .img that are usually RAW images --- libvirt-sandbox/Makefile.am| 1 + libvirt-sandbox/libvirt-sandbox-util.c | 72 ++ libvirt-sandbox/libvirt-sandbox-util.h | 5 +++ po/POTFILES.in | 1 + 4 files changed, 79 insertions(+) create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 96302cb..6917f04 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -84,6 +84,7 @@ SANDBOX_HEADER_FILES = \ $(NULL) SANDBOX_SOURCE_FILES = \ libvirt-sandbox-main.c \ + libvirt-sandbox-util.c \ libvirt-sandbox-config.c \ libvirt-sandbox-config-network.c \ libvirt-sandbox-config-network-address.c \ diff --git a/libvirt-sandbox/libvirt-sandbox-util.c b/libvirt-sandbox/libvirt-sandbox-util.c new file mode 100644 index 000..32d4c4e --- /dev/null +++ b/libvirt-sandbox/libvirt-sandbox-util.c @@ -0,0 +1,72 @@ +/* + * libvirt-sandbox-util.c: libvirt sandbox util functions + * + * Copyright (C) 2015 Universitat Polit??cnica de Catalunya. + * + * 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 even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: Eren Yagdiran erenyagdi...@gmail.com + */ + +#include config.h +#include string.h +#include errno.h +#include glib/gi18n.h + +#include libvirt-sandbox/libvirt-sandbox.h + +#define GVIR_SANDBOX_UTIL_ERROR gvir_sandbox_util_error_quark() + +static GQuark +gvir_sandbox_util_error_quark(void) +{ +return g_quark_from_static_string(gvir-sandbox-util); +} + +gint gvir_sandbox_util_guess_image_format(const gchar *path, + GError **error) +{ +gchar *tmp; + +if ((tmp = strchr(path, '.')) == NULL) { +return -1; +} +tmp = tmp + 1; + +if (strcmp(tmp,img)==0){ + return GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW; +} + +return gvir_sandbox_util_disk_format_from_str(tmp, error); +} + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value, +GError **error) +{ +GEnumClass *enum_class = g_type_class_ref(GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT); +GEnumValue *enum_value; +gint ret = -1; + +if (!(enum_value = g_enum_get_value_by_nick(enum_class, value))) { +g_set_error(error, GVIR_SANDBOX_UTIL_ERROR, 0, +_(Unknown disk format '%s'), value); +goto cleanup; +} +ret = enum_value-value; + + cleanup: +g_type_class_unref(enum_class); +return ret; +} diff --git a/libvirt-sandbox/libvirt-sandbox-util.h b/libvirt-sandbox/libvirt-sandbox-util.h index ae6b74b..890ae7f 100644 --- a/libvirt-sandbox/libvirt-sandbox-util.h +++ b/libvirt-sandbox/libvirt-sandbox-util.h @@ -29,6 +29,11 @@ G_BEGIN_DECLS +gint gvir_sandbox_util_guess_image_format(const gchar *path, GError **error); + + +gint gvir_sandbox_util_disk_format_from_str(const gchar *value, GError **error); + /** * LIBVIRT_SANDBOX_CLASS_PADDING: (skip) */ diff --git a/po/POTFILES.in b/po/POTFILES.in index 653abc5..f291b98 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -9,3 +9,4 @@ libvirt-sandbox/libvirt-sandbox-console-rpc.c libvirt-sandbox/libvirt-sandbox-context.c libvirt-sandbox/libvirt-sandbox-init-common.c libvirt-sandbox/libvirt-sandbox-rpcpacket.c +libvirt-sandbox/libvirt-sandbox-util.c -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 08/10] Init-util : Common directory functions for init-common and init-qemu
Init-util provides common functions for creating directories for both Init-common and Init-qemu. Error handling needs to be done in calling function. --- libvirt-sandbox/Makefile.am | 4 +- libvirt-sandbox/libvirt-sandbox-init-qemu.c | 57 +--- libvirt-sandbox/libvirt-sandbox-init-util.c | 58 + libvirt-sandbox/libvirt-sandbox-init-util.h | 41 4 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.c create mode 100644 libvirt-sandbox/libvirt-sandbox-init-util.h diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am index 8237c50..a7f1232 100644 --- a/libvirt-sandbox/Makefile.am +++ b/libvirt-sandbox/Makefile.am @@ -166,6 +166,7 @@ libvirt_sandbox_1_0_la_LDFLAGS = \ -version-info $(LIBVIRT_SANDBOX_VERSION_INFO) libvirt_sandbox_init_common_SOURCES = libvirt-sandbox-init-common.c \ + libvirt-sandbox-init-util.c \ $(SANDBOX_GENERATED_RPC_FILES) \ $(SANDBOX_RPC_FILES) \ $(NULL) @@ -216,7 +217,8 @@ libvirt_sandbox_init_lxc_LDADD = \ libvirt-sandbox-1.0.la \ $(NULL) -libvirt_sandbox_init_qemu_SOURCES = libvirt-sandbox-init-qemu.c +libvirt_sandbox_init_qemu_SOURCES = libvirt-sandbox-init-qemu.c \ + libvirt-sandbox-init-util.c libvirt_sandbox_init_qemu_CFLAGS = \ -DLIBEXECDIR=\$(libexecdir)\ \ -DSANDBOXCONFIGDIR=\$(sandboxconfigdir)\ \ diff --git a/libvirt-sandbox/libvirt-sandbox-init-qemu.c b/libvirt-sandbox/libvirt-sandbox-init-qemu.c index 33cebed..aa1a092 100644 --- a/libvirt-sandbox/libvirt-sandbox-init-qemu.c +++ b/libvirt-sandbox/libvirt-sandbox-init-qemu.c @@ -43,6 +43,8 @@ #include sys/reboot.h #include termios.h +#include libvirt-sandbox-init-util.h + #define ATTR_UNUSED __attribute__((__unused__)) static void print_uptime (void); @@ -75,43 +77,13 @@ static void sig_child(int signum ATTR_UNUSED) } static void -mount_mkdir(const char *dir, int mode); - -static void -mount_mkparent(const char *dir, int mode) -{ -char *tmp = strrchr(dir, '/'); -if (tmp tmp != dir) { -char *parent = strndup(dir, tmp - dir); -mount_mkdir(parent, mode); -free(parent); -} -} - -static void -mount_mkdir(const char *dir, int mode) -{ -mount_mkparent(dir, 0755); - -if (debug) -fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s (mode=%o)\n, __func__, dir, mode); - -if (mkdir(dir, mode) 0) { -if (errno != EEXIST) { -fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot make directory %s: %s\n, -__func__, dir, strerror(errno)); -exit_poweroff(); -} -} -} - -static void mount_mkfile(const char *file, int mode) { int fd; -mount_mkparent(file, 0755); - +if (gvir_sandbox_init_util_mkparent(file, 0755, debug) 0) + exit_poweroff(); + if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s (mode=%o)\n, __func__, file, mode); @@ -132,8 +104,9 @@ mount_other_opts(const char *dst, const char *type, const char *opts, int mode) if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s (type=%s opts=%s)\n, __func__, dst, type, opts); -mount_mkdir(dst, mode); - +if (gvir_sandbox_init_util_mkdir(dst, mode,debug) 0) + exit_poweroff(); + if (mount(none, dst, type, 0, opts) 0) { fprintf(stderr, libvirt-sandbox-init-qemu: %s: cannot mount %s on %s (%s:%s): %s\n, __func__, type, dst, type, opts, strerror(errno)); @@ -160,8 +133,9 @@ mount_9pfs(const char *src, const char *dst, int mode, int readonly) if (debug) fprintf(stderr, libvirt-sandbox-init-qemu: %s: %s - %s (%d)\n, __func__, src, dst, readonly); -mount_mkdir(dst, mode); - +if (gvir_sandbox_init_util_mkdir(dst, mode, debug) 0) +exit_poweroff(); + if (readonly) flags |= MS_RDONLY; @@ -286,15 +260,18 @@ main(int argc ATTR_UNUSED, char **argv ATTR_UNUSED) __func__, source, strerror(errno)); exit_poweroff(); } -if (S_ISDIR(st.st_mode)) -mount_mkdir(target, 755); +if (S_ISDIR(st.st_mode)){ +if (gvir_sandbox_init_util_mkdir(target, 755, debug) 0) + exit_poweroff(); + } else mount_mkfile(target, 644); } else { if (strcmp(type, tmpfs) == 0) flags |= MS_NOSUID | MS_NODEV; -mount_mkdir(target, 0755); +if (gvir_sandbox_init_util_mkdir(target, 0755, debug) 0) +exit_poweroff(); } if (mount(source, target, type, flags,
Re: [libvirt] [PATCH 1/7] qemu: Introduce qemuIsSharedHostdev
On 06/16/2015 03:51 PM, John Ferlan wrote: Add a single boolean function to handle whether the hostdev is shared or not Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_conf.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) Given Jan's comments about the overall series, I'd like to propose squashing in the attached patch into this patch. Essentially it's making the same check and returning 0 if the shared bit wasn't set. The current code checks for shared hostdev *and* sgio is set, then fail; otherwise, return 0. This patch will check shared hostdev, return 0 if not.. then check sgio singularly and fail; otherwise return 0. John diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 16ae6ab..8e9da0d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1196,6 +1196,19 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, } +static bool +qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev) +{ +if (hostdev-shareable +(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS + hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI + hostdev-source.subsys.u.scsi.protocol != + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) +return true; +return false; +} + + static char * qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev) { @@ -1233,10 +1246,7 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver, char *key = NULL; int ret = -1; -if (!hostdev-shareable || -!(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS - hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI - hostdev-source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) +if (!qemuIsSharedHostdev(hostdev)) return 0; if (!(key = qemuGetSharedHostdevKey(hostdev))) @@ -1337,10 +1347,7 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver, char *key = NULL; int ret; -if (!hostdev-shareable || -!(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS - hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI - hostdev-source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) +if (!qemuIsSharedHostdev(hostdev)) return 0; if (!(key = qemuGetSharedHostdevKey(hostdev))) From c8e7805d57c2ce2f8bd0a41ef0dbdcbc359a1223 Mon Sep 17 00:00:00 2001 From: John Ferlan jfer...@redhat.com Date: Wed, 24 Jun 2015 10:32:38 -0400 Subject: [PATCH 02/10] Merge into patch 1 Signed-off-by: John Ferlan jfer...@redhat.com --- src/qemu/qemu_conf.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 305cfb5..48fb74a 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1414,11 +1414,10 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev-data.hostdev; +if (!qemuIsSharedHostdev(hostdev)) +return 0; -if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS -hostdev-source.subsys.type == -VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI -hostdev-source.subsys.u.scsi.sgio) { +if (hostdev-source.subsys.u.scsi.sgio) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _('sgio' is not supported for SCSI generic device yet )); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 05/10] Add disk support to machine builder
Use the new disk configuration in the container builder to provide disks in qemu sandboxes. The disks are virtio devices, but those shouldn't be known by the user. --- libvirt-sandbox/libvirt-sandbox-builder-machine.c | 38 --- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-machine.c b/libvirt-sandbox/libvirt-sandbox-builder-machine.c index 5e6bf72..4148d4b 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-machine.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-machine.c @@ -174,7 +174,8 @@ static gchar *gvir_sandbox_builder_machine_mkinitrd(GVirSandboxConfig *config, if (gvir_sandbox_config_has_networks(config)) gvir_sandbox_config_initrd_add_module(initrd, virtio_net.ko); if (gvir_sandbox_config_has_mounts_with_type(config, - GVIR_SANDBOX_TYPE_CONFIG_MOUNT_HOST_IMAGE)) + GVIR_SANDBOX_TYPE_CONFIG_MOUNT_HOST_IMAGE) || +gvir_sandbox_config_has_disks(config)) gvir_sandbox_config_initrd_add_module(initrd, virtio_blk.ko); gvir_sandbox_config_initrd_add_module(initrd, virtio_console.ko); #if 0 @@ -503,9 +504,9 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde GVirConfigDomainConsole *con; GVirConfigDomainSerial *ser; GVirConfigDomainChardevSourcePty *src; -GList *tmp = NULL, *mounts = NULL, *networks = NULL; +GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; size_t nHostBind = 0; -size_t nHostImage = 0; +size_t nVirtioDev = 0; gchar *configdir = g_strdup_printf(%s/config, statedir); gboolean ret = FALSE; @@ -538,6 +539,35 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde g_object_unref(fs); +tmp = disks = gvir_sandbox_config_get_disks(config); +while(tmp){ +GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); + +if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ +gchar *device = g_strdup_printf(vd%c, (char)('a' + nVirtioDev++)); + +disk = gvir_config_domain_disk_new(); +diskDriver = gvir_config_domain_disk_driver_new(); +gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); +gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); +gvir_config_domain_disk_set_source(disk, gvir_sandbox_config_disk_get_source(dconfig)); +gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO); +gvir_config_domain_disk_set_target_dev(disk, + device); +gvir_config_domain_disk_set_driver(disk, diskDriver); +gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(disk)); +g_object_unref(disk); +g_free(device); +} +tmp = tmp-next; +} + +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); + tmp = mounts = gvir_sandbox_config_get_mounts(config); while (tmp) { GVirSandboxConfigMount *mconfig = GVIR_SANDBOX_CONFIG_MOUNT(tmp-data); @@ -563,7 +593,7 @@ static gboolean gvir_sandbox_builder_machine_construct_devices(GVirSandboxBuilde GVirSandboxConfigMountFile *mfile = GVIR_SANDBOX_CONFIG_MOUNT_FILE(mconfig); GVirSandboxConfigMountHostImage *mimage = GVIR_SANDBOX_CONFIG_MOUNT_HOST_IMAGE(mconfig); GVirConfigDomainDiskFormat format; -gchar *target = g_strdup_printf(vd%c, (char)('a' + nHostImage++)); +gchar *target = g_strdup_printf(vd%c, (char)('a' + nVirtioDev++)); disk = gvir_config_domain_disk_new(); gvir_config_domain_disk_set_type(disk, GVIR_CONFIG_DOMAIN_DISK_FILE); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 04/10] Add disk support to the container builder
Use the new disk configuration in the container builder to provide disks in lxc containers sandboxes. --- .../libvirt-sandbox-builder-container.c| 36 +- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c b/libvirt-sandbox/libvirt-sandbox-builder-container.c index 59bfee1..2a6c273 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder-container.c +++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c @@ -218,7 +218,9 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil GVirConfigDomainInterfaceNetwork *iface; GVirConfigDomainConsole *con; GVirConfigDomainChardevSourcePty *src; -GList *tmp = NULL, *mounts = NULL, *networks = NULL; +GVirConfigDomainDisk *disk; +GVirConfigDomainDiskDriver *diskDriver; +GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL; gchar *configdir = g_strdup_printf(%s/config, statedir); gboolean ret = FALSE; @@ -226,6 +228,38 @@ static gboolean gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil construct_devices(builder, config, statedir, domain, error)) goto cleanup; + +tmp = disks = gvir_sandbox_config_get_disks(config); +while(tmp){ +GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp-data); + +if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){ + +disk = gvir_config_domain_disk_new(); +diskDriver = gvir_config_domain_disk_driver_new(); +gvir_config_domain_disk_set_type(disk, + gvir_sandbox_config_disk_get_disk_type(dconfig)); +gvir_config_domain_disk_driver_set_format(diskDriver, + gvir_sandbox_config_disk_get_format(dconfig)); +gvir_config_domain_disk_driver_set_name(diskDriver, nbd); +gvir_config_domain_disk_set_source(disk, + gvir_sandbox_config_disk_get_source(dconfig)); +gvir_config_domain_disk_set_target_bus(disk, + GVIR_CONFIG_DOMAIN_DISK_BUS_IDE); +gvir_config_domain_disk_set_target_dev(disk, + gvir_sandbox_config_disk_get_target(dconfig)); +gvir_config_domain_disk_set_driver(disk, diskDriver); +gvir_config_domain_add_device(domain, + GVIR_CONFIG_DOMAIN_DEVICE(disk)); +g_object_unref(disk); +} +tmp = tmp-next; +} + +g_list_foreach(disks, (GFunc)g_object_unref, NULL); +g_list_free(disks); + + fs = gvir_config_domain_filesys_new(); gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_MOUNT); gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_PASSTHROUGH); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] vz: implementation of attach/detach network devices
On 06/22/2015 07:57 PM, Mikhail Feoktistov wrote: In this patch we add VIR_DOMAIN_DEVICE_NET handlers implementation for domainAttachDevice and domainDetachDevice callbacks. As soon as we don't support this operation for hypervisor type domains, we implement this functionality for containers only. In detach procedure we find network device by MAC address. Because PrlVmDevNet_GetMacAddress() returns MAC as a UTF-8 encoded null-terminated string, we use memcmp() to compare it. Also we remove corresponding virtual network by prlsdkDelNetAdapter call. I've fixed small syntax lacks and pushed the patch. --- src/vz/vz_driver.c | 16 +++ src/vz/vz_sdk.c| 123 src/vz/vz_sdk.h|4 ++ 3 files changed, 143 insertions(+), 0 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index cef3c77..d9ddd4f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1117,6 +1117,14 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkAttachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network attach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be attached), @@ -1186,6 +1194,14 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } break; +case VIR_DOMAIN_DEVICE_NET: +ret = prlsdkDetachNet(privdom, privconn, dev-data.net); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network detach failed)); +goto cleanup; +} +break; default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _(device type '%s' cannot be detached), diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 090a3ad..6e6d8c9 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2914,6 +2914,129 @@ static void prlsdkDelNet(vzConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); } +int prlsdkAttachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) Too long line +{ +int ret = -1; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +if (!IS_CT(dom-def)) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, + _(network device cannot be attached)); +return ret; +} + +job = PrlVm_BeginEdit(privdom-sdkdom); +if (PRL_FAILED(waitJob(job))) +return ret; + +ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def)); +if (ret == 0) { +job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); +if (PRL_FAILED(waitJob(job))) { +return -1; +} Braces around single-statement block. +} + +return ret; +} + +static int +prlsdkGetNetIndex(PRL_HANDLE sdkdom, virDomainNetDefPtr net) +{ +int idx = -1; +PRL_RESULT pret; +PRL_UINT32 adaptersCount; +PRL_UINT32 i; +PRL_HANDLE adapter = PRL_INVALID_HANDLE; +PRL_UINT32 len; +char adapterMac[PRL_MAC_STRING_BUFNAME]; +char expectedMac[PRL_MAC_STRING_BUFNAME]; + +prlsdkFormatMac(net-mac, expectedMac); +pret = PrlVmCfg_GetNetAdaptersCount(sdkdom, adaptersCount); +prlsdkCheckRetGoto(pret, cleanup); + +for (i = 0; i adaptersCount; ++i) { + +pret = PrlVmCfg_GetNetAdapter(sdkdom, i, adapter); +prlsdkCheckRetGoto(pret, cleanup); + +len = sizeof(adapterMac); +memset(adapterMac, 0, sizeof(adapterMac)); +pret = PrlVmDevNet_GetMacAddress(adapter, adapterMac, len); +prlsdkCheckRetGoto(pret, cleanup); + +if (memcmp(adapterMac, expectedMac, PRL_MAC_STRING_BUFNAME)) { + +PrlHandle_Free(adapter); +adapter = PRL_INVALID_HANDLE; +continue; +} + +idx = i; +break; +} + + cleanup: +PrlHandle_Free(adapter); +return idx; +} + +static int prlsdkDelNetAdapter(PRL_HANDLE sdkdom, int idx) +{ +int ret = -1; +PRL_RESULT pret; +PRL_HANDLE sdknet = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetNetAdapter(sdkdom, idx, sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_Remove(sdknet); +prlsdkCheckRetGoto(pret, cleanup); + +ret = 0; + + cleanup: +PrlHandle_Free(sdknet); +return ret; +} + +int prlsdkDetachNet(virDomainObjPtr dom, parallelsConnPtr privconn, virDomainNetDefPtr net) Too long line +{ +int ret = -1, idx = -1; +parallelsDomObjPtr privdom = dom-privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +if (!IS_CT(dom-def)) { +
Re: [libvirt] [PATCH v2 13/15] test: Refactor testDomainSetVcpusFlags
On Wed, Jun 24, 2015 at 04:11:55PM +0200, Peter Krempa wrote: Remove the bogous flag check and refactor the code by using s/bogous/bogus/ virDomainObjGetDefs instead of virDomainObjGetPersistentDef. --- src/test/test_driver.c | 68 +- 1 file changed, 23 insertions(+), 45 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2c2f009..ae332ef 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2445,6 +2445,7 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, { testDriverPtr privconn = domain-conn-privateData; virDomainObjPtr privdom = NULL; +virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1, maxvcpus; @@ -2452,72 +2453,49 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); -/* At least one of LIVE or CONFIG must be set. MAXIMUM cannot be - * mixed with LIVE. */ -if ((flags (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == 0 || -(flags (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) == - (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) { -virReportError(VIR_ERR_INVALID_ARG, - _(invalid flag combination: (0x%x)), flags); +if ((maxvcpus = testConnectGetMaxVcpus(domain-conn, NULL)) == -1) return -1; -} -if (!nrCpus || (maxvcpus = testConnectGetMaxVcpus(domain-conn, NULL)) nrCpus) { + +if (nrCpus maxvcpus) { virReportError(VIR_ERR_INVALID_ARG, - _(argument out of range: %d), nrCpus); + _(requested cpu amount exceeds maximum supported amount + (%d %d)), nrCpus, maxvcpus); return -1; } if (!(privdom = testDomObjFromDomain(domain))) return -1; -if (!virDomainObjIsActive(privdom) (flags VIR_DOMAIN_AFFECT_LIVE)) { -virReportError(VIR_ERR_OPERATION_INVALID, - %s, _(cannot hotplug vcpus for an inactive domain)); +if (virDomainObjGetDefs(privdom, flags, def, persistentDef) 0) goto cleanup; -} - -/* We allow more cpus in guest than host, but not more than the - * domain's starting limit. */ -if (!(flags (VIR_DOMAIN_VCPU_MAXIMUM)) -privdom-def-maxvcpus maxvcpus) -maxvcpus = privdom-def-maxvcpus; -if (nrCpus maxvcpus) { +if ((def + def-maxvcpus nrCpus) || +(persistentDef + !(flags VIR_DOMAIN_VCPU_MAXIMUM) + persistentDef-maxvcpus nrCpus)) { virReportError(VIR_ERR_INVALID_ARG, _(requested cpu amount exceeds maximum (%d %d)), nrCpus, maxvcpus); As pointed in the previous review, you need to use (def|persistentDef)-maxvcpus in the error message. goto cleanup; } -if (!(persistentDef = virDomainObjGetPersistentDef(privconn-caps, - privconn-xmlopt, - privdom))) +if (def +testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0) 0) goto cleanup; -switch (flags) { -case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_CONFIG: -persistentDef-maxvcpus = nrCpus; -if (nrCpus persistentDef-vcpus) -persistentDef-vcpus = nrCpus; -ret = 0; -break; - -case VIR_DOMAIN_AFFECT_CONFIG: -persistentDef-vcpus = nrCpus; -ret = 0; -break; - -case VIR_DOMAIN_AFFECT_LIVE: -ret = testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0); -break; - -case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG: -ret = testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0); -if (ret == 0) +if (persistentDef) { +if (flags VIR_DOMAIN_VCPU_MAXIMUM) { +persistentDef-maxvcpus = nrCpus; +if (nrCpus persistentDef-vcpus) +persistentDef-vcpus = nrCpus; +} else { persistentDef-vcpus = nrCpus; -break; +} } +ret = 0; + cleanup: virDomainObjEndAPI(privdom); return ret; -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote: I think the only votes were for option 1 and 4 (interesting how the only ones that were chosen were those that I *didn't* pick personally :-). See comments below. In the meantime the other issue Alex pointed out may cause this to take a slightly different direction. On 06/22/2015 02:44 PM, Laine Stump wrote: === Idea 1: multiplex the meaning of the model attribute, so we currently have: controller type='pci' model='dmi-to-pci-bridge'/ which means add an i82801b11-bridge device and when we add a generic version of this type of controller, we would do it with something like: controller type='pci' model='generic-dmi-to-pci-bridge'/ and for another vendor's mythical controller: controller type='pci' model='xyz-dmi-to-pci-bridge'/ Cons: This will make for ugliness in switch statements where a new case will have to be added whenever a different controller with similar behavior/usage is supported. And it's generally not a good idea to have a single attribute be used for two different functions. jtomko advocated this one because it would make it easier for an older libvirt to notice an unsupported class+model of controller during a migration attempt from a newer libvirt. An example would be if a newer libvirt had a guest with controller type='pci' model='xyz'/ (where xyz is some qemu device that implements a dmi-to-pci-bridge) That would fail on an attempt to migrate to older libvirt, but controller type='pci' model='dmi-to-pci-bridge'/ would succeed. On the other hand, if we did this: controller type='pci' model='dmi-to-pci-bridge submodel='xyz'/ that would succeed (and create the wrong device) because submodel would be ignored. Since there is currently no alternate implementation of a dmi-to-pci-bridge available in qemu, and it will likely be awhile until that happens, I think if we start filling out the XML now as: controller type='pci' model='dmi-to-pci-bridge' submodel='i82801b11-bridge'/ by the time we get to the point that we do have an alternate 'xyz' controller, any version of libvirt that doesn't understand submodel will be so far in the past that we wouldn't want to be able to migrate back to it anyway. === Idea 4: Unlike other uses of model in libvirt, for pci controllers, continue to use model to denote the subtype/class/whatever of controller, and create a new attribute to denote the different specific implementations of each one. So for example: [4] controller type='pci' model='dmi-to-pci-bridge'/ would become: [5] controller type='pci' model='dmi-to-pci-bridge' implementation='i82801b11-bridge'/ (or some other name in place of implementation - ideas? I'm horrible at thinkin up names) Pros: wouldn't create compatibility problems when downgrading or migrating cross version. Cons: Is inconsistent with every other use of model attribute in libvirt, and each new addition of a PCI controller further propagates this misuse. mkletzan preferred this one, and danpb was amenable to it in IRC. I think I now agree, but Alex's comments about needing to keep track of what we put in the qemu commandline for port and chassis have me thinking this isn't enough. The problem is that the ioh3420 device needs its port and chassis options set; Alex recommends setting port to: (slot 3)+function Likewise, he recommends setting port for the xio3130-downstream device to the same value as slot. So, for the following: controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' /controller we would end up with the following commandline: -device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0', addr=0x4.0x1 (chassis is the same as index in the xml (again per Alex's recommendation), and port is (4 3) + 1 = 0x21) *However* he also says that we may change our mind on these in the future, so chassis and port may end up being something different. Since those values are visible to the guest, we can't allow them to change on an existing machine, as it breaks the guest ABI. So we need to store them in the XML. They aren't part of the PCI address though, they are options. So I need to figure out the best way to represent that in the XML, and fill it in when it is auto-generated when the controller is defined. A few possible ways to solve both problems at once: [1] controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' target type='ioh3420' chassis='3' port='0x21'/ /controller Precedent: target is used to store the port number for serial/console devices, specify guest-side bus and device name for disks, specify guest-side mount location for filesystems, specify the *host*-side
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
- On 24 Jun, 2015, at 09:14, Martin Kletzander mklet...@redhat.com wrote: Why don't you just set XMLFILE=$TMPFILE ? That would get rid of lot of the code changes below and would be more readable. Other than that it looks good. Good question, with a somewhat long answer. I initially did exactly what you suggest, and yes, it makes for simpler code. The only problem with it is that when xmllint finds a problem with the xml, it will output a string on the format file name:line number: error description. If the file was provided on stdin, the file name will be something like /tmp/virt-xml.asdf which will not make much sense to the user. So instead I tried to be consistent: if virt-xml-validate received a filename, xmllint will receive a filename. If virt-xml-validate received data on stdin, xmllint will receive data on stdin. This way, when the xml is provided on stdin and xmllint finds a problem, the file name in the error message will just show up as -, which is probably more in line with what the user was expecting. Anyway, it's not something I feel super-strongly about, so if you prefer to have it changed, just let me know and I'll have an updated patch for you. /Johannes -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] PCIe fixes + new PCI controllers w/RFC
I think the only votes were for option 1 and 4 (interesting how the only ones that were chosen were those that I *didn't* pick personally :-). See comments below. In the meantime the other issue Alex pointed out may cause this to take a slightly different direction. On 06/22/2015 02:44 PM, Laine Stump wrote: === Idea 1: multiplex the meaning of the model attribute, so we currently have: controller type='pci' model='dmi-to-pci-bridge'/ which means add an i82801b11-bridge device and when we add a generic version of this type of controller, we would do it with something like: controller type='pci' model='generic-dmi-to-pci-bridge'/ and for another vendor's mythical controller: controller type='pci' model='xyz-dmi-to-pci-bridge'/ Cons: This will make for ugliness in switch statements where a new case will have to be added whenever a different controller with similar behavior/usage is supported. And it's generally not a good idea to have a single attribute be used for two different functions. jtomko advocated this one because it would make it easier for an older libvirt to notice an unsupported class+model of controller during a migration attempt from a newer libvirt. An example would be if a newer libvirt had a guest with controller type='pci' model='xyz'/ (where xyz is some qemu device that implements a dmi-to-pci-bridge) That would fail on an attempt to migrate to older libvirt, but controller type='pci' model='dmi-to-pci-bridge'/ would succeed. On the other hand, if we did this: controller type='pci' model='dmi-to-pci-bridge submodel='xyz'/ that would succeed (and create the wrong device) because submodel would be ignored. Since there is currently no alternate implementation of a dmi-to-pci-bridge available in qemu, and it will likely be awhile until that happens, I think if we start filling out the XML now as: controller type='pci' model='dmi-to-pci-bridge' submodel='i82801b11-bridge'/ by the time we get to the point that we do have an alternate 'xyz' controller, any version of libvirt that doesn't understand submodel will be so far in the past that we wouldn't want to be able to migrate back to it anyway. === Idea 4: Unlike other uses of model in libvirt, for pci controllers, continue to use model to denote the subtype/class/whatever of controller, and create a new attribute to denote the different specific implementations of each one. So for example: [4] controller type='pci' model='dmi-to-pci-bridge'/ would become: [5] controller type='pci' model='dmi-to-pci-bridge' implementation='i82801b11-bridge'/ (or some other name in place of implementation - ideas? I'm horrible at thinkin up names) Pros: wouldn't create compatibility problems when downgrading or migrating cross version. Cons: Is inconsistent with every other use of model attribute in libvirt, and each new addition of a PCI controller further propagates this misuse. mkletzan preferred this one, and danpb was amenable to it in IRC. I think I now agree, but Alex's comments about needing to keep track of what we put in the qemu commandline for port and chassis have me thinking this isn't enough. The problem is that the ioh3420 device needs its port and chassis options set; Alex recommends setting port to: (slot 3)+function Likewise, he recommends setting port for the xio3130-downstream device to the same value as slot. So, for the following: controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' /controller we would end up with the following commandline: -device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0', addr=0x4.0x1 (chassis is the same as index in the xml (again per Alex's recommendation), and port is (4 3) + 1 = 0x21) *However* he also says that we may change our mind on these in the future, so chassis and port may end up being something different. Since those values are visible to the guest, we can't allow them to change on an existing machine, as it breaks the guest ABI. So we need to store them in the XML. They aren't part of the PCI address though, they are options. So I need to figure out the best way to represent that in the XML, and fill it in when it is auto-generated when the controller is defined. A few possible ways to solve both problems at once: [1] controller type='pci model='pci-root-port' index='3' address type='pci' bus='0' slot='4' function='1' target type='ioh3420' chassis='3' port='0x21'/ /controller Precedent: target is used to store the port number for serial/console devices, specify guest-side bus and device name for disks, specify guest-side mount location for filesystems, specify the *host*-side name of tap devices for network interfaces, memory size for memory. So it seems kind of proper. (slight variation - target model='ioh3420' .../ :-/ ) [2] controller type='pci
[libvirt] Build failed in Jenkins: libvirt-syntax-check #3634
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/3634/ -- Started by upstream project libvirt-build build number 4138 Building on master in workspace http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/ [workspace] $ /bin/sh -xe /tmp/hudson3337865490993241299.sh + make syntax-check GENbracket-spacing-check Curly brackets around single-line body: src/vz/vz_sdk.c:2932-2934: if (PRL_FAILED(waitJob(job))) { return -1; } Curly brackets around single-line body: src/vz/vz_sdk.c:3030-3032: if (PRL_FAILED(waitJob(job))) { return -1; } maint.mk: incorrect formatting, see HACKING for rules make: *** [bracket-spacing-check] Error 1 Build step 'Execute shell' marked build as failure -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 10/15] test: Finalize removal of locking from driver-eventState
On 06/24/2015 10:11 AM, Peter Krempa wrote: Don't lock the driver when registering event callbacks. --- src/test/test_driver.c | 12 1 file changed, 12 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2ab0402..b08c1e5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5915,11 +5915,9 @@ testConnectDomainEventRegister(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virDomainEventStateRegister(conn, driver-eventState, callback, opaque, freecb) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5932,11 +5930,9 @@ testConnectDomainEventDeregister(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virDomainEventStateDeregister(conn, driver-eventState, callback) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5953,12 +5949,10 @@ testConnectDomainEventRegisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret; int ret = 0; -testDriverLock(driver); if (virDomainEventStateRegisterID(conn, driver-eventState, dom, eventID, callback, opaque, freecb, ret) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5970,11 +5964,9 @@ testConnectDomainEventDeregisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virObjectEventStateDeregisterID(conn, driver-eventState, callbackID) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -5991,12 +5983,10 @@ testConnectNetworkEventRegisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret; int ret = 0; I know, existing... but for consistency of other places... not required, but suggested ;-) John -testDriverLock(driver); if (virNetworkEventStateRegisterID(conn, driver-eventState, net, eventID, callback, opaque, freecb, ret) 0) ret = -1; -testDriverUnlock(driver); return ret; } @@ -6008,11 +5998,9 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn, testDriverPtr driver = conn-privateData; int ret = 0; -testDriverLock(driver); if (virObjectEventStateDeregisterID(conn, driver-eventState, callbackID) 0) ret = -1; -testDriverUnlock(driver); return ret; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0.5/7] docs: Remove sgio from hostdev docs
Since hostdev sgio is not supported, let's remove it from the docs to remove confusion. Signed-off-by: John Ferlan jfer...@redhat.com --- Hopefully I got the in-reply-to magic correct! I figured keeping the sgio buried in the scsisrc is ok for those down stream providers that want or have the ability to support it. I could remove it as well if that's desired. docs/formatdomain.html.in | 23 +-- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f7b9f51..59b4e86 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3171,23 +3171,6 @@ pre ... lt;devicesgt; -lt;hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'gt; - lt;sourcegt; -lt;adapter name='scsi_host0'/gt; -lt;address bus='0' target='0' unit='0'/gt; - lt;/sourcegt; - lt;readonly/gt; - lt;address type='drive' controller='0' bus='0' target='0' unit='0'/gt; -lt;/hostdevgt; - lt;/devicesgt; - .../pre - - -por:/p - -pre - ... - lt;devicesgt; lt;hostdev mode='subsystem' type='scsi'gt; lt;source protocol='iscsi' name='iqn.2014-08.com.example:iscsi-nopool/1'gt; lt;host name='example.com' port='3260'/gt; @@ -3225,11 +3208,7 @@ /dd dtscsi/dt ddFor SCSI devices, user is responsible to make sure the device -is not used by host. The optional codesgio/code -(span class=sincesince 1.0.6/span) attribute indicates -whether the kernel will filter unprivileged SG_IO commands for -the disk, valid settings are filtered or unfiltered. -The default is filtered. The optional coderawio/code +is not used by host. The optional coderawio/code (span class=sincesince 1.2.9/span) attribute indicates whether the lun needs the rawio capability. Valid settings are yes or no. See the rawio description within the -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vz: fix syntax-check errors
Remove braces around single-statement blocks in vz_sdk.c Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/vz/vz_sdk.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index dea6e37..1a3aa87 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2929,9 +2929,8 @@ int prlsdkAttachNet(virDomainObjPtr dom, ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def)); if (ret == 0) { job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); -if (PRL_FAILED(waitJob(job))) { +if (PRL_FAILED(waitJob(job))) return -1; -} } return ret; @@ -3027,9 +3026,8 @@ int prlsdkDetachNet(virDomainObjPtr dom, ret = prlsdkDelNetAdapter(privdom-sdkdom, idx); if (ret == 0) { job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); -if (PRL_FAILED(waitJob(job))) { +if (PRL_FAILED(waitJob(job))) return -1; -} } return ret; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vz: fix syntax-check errors
On 06/24/2015 10:37 PM, Dmitry Guryanov wrote: Remove braces around single-statement blocks in vz_sdk.c Pushed as trivial and build-breaker. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/vz/vz_sdk.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index dea6e37..1a3aa87 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2929,9 +2929,8 @@ int prlsdkAttachNet(virDomainObjPtr dom, ret = prlsdkAddNet(privdom-sdkdom, privconn, net, IS_CT(dom-def)); if (ret == 0) { job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); -if (PRL_FAILED(waitJob(job))) { +if (PRL_FAILED(waitJob(job))) return -1; -} } return ret; @@ -3027,9 +3026,8 @@ int prlsdkDetachNet(virDomainObjPtr dom, ret = prlsdkDelNetAdapter(privdom-sdkdom, idx); if (ret == 0) { job = PrlVm_CommitEx(privdom-sdkdom, PVCF_DETACH_HDD_BUNDLE); -if (PRL_FAILED(waitJob(job))) { +if (PRL_FAILED(waitJob(job))) return -1; -} } return ret; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libvirt/hooks: Static and dynamic hugepage hooks
This patch provides scripts for hugepage allocation, as well as a bit of infrastructure and common hook config file that I hope may some day be enabled by default in libvirt. For now, we place the files in /usr/share and ask users to install the config file and copy or link the scripts, more like contrib scripts for now. Two methods of hugepage allocation are provided, static and dynamic. The static mechanism allocates pages at libvirt daemon startup and releases them at shutdown. It allows full size, locality, and policy configuration. For instance, if I want to allocate a set of 2M pages exclusively on host NUMA node 1, it can do that, along with plenty more. This is especially useful for 1G hugepages on x86, since they can now be allocated dynamically, but become impractical to allocate due to memory fragmentation as the host runs. Systems dedicated to hosting VMs are also likely to prefer static allocation. Static allocation requires explicit XML entries in the hook config file to be activated. The dynamic method allocates hugepages only around the instantiation of the VM. This is enabled by adding an entry for the domain in the config file and configuring the domain normally for hugepages. The dynamic hugepage script is activated via the QEMU domain prepare hook, reads the domain XML and allocates hugepages as necessary. On domain shutdown, hugepages are freed via the release hook. This model is more appropriate for systems that are not dedicated VM hosts and guests that use hugepage sizes and quantities are are likely to be dynamically allocated as the VM is started. In addition to the documentation provided within each script, a README file is provided with overal instructions and summaries of the individual scripts. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Makefile.am|2 configure.ac |3 hooks/Makefile.am | 27 hooks/README | 47 +++ hooks/daemon | 33 + hooks/daemon.d/Makefile.am | 21 +++ hooks/daemon.d/static-hugepages.sh | 100 +++ hooks/functions.sh | 180 +++ hooks/libvirt-hook-config.xml | 38 ++ hooks/qemu | 40 ++ hooks/qemu.d/Makefile.am | 21 +++ hooks/qemu.d/dynamic-hugepages.sh | 238 libvirt.spec.in| 13 ++ 13 files changed, 761 insertions(+), 2 deletions(-) create mode 100644 hooks/Makefile.am create mode 100644 hooks/README create mode 100755 hooks/daemon create mode 100644 hooks/daemon.d/Makefile.am create mode 100755 hooks/daemon.d/static-hugepages.sh create mode 100755 hooks/functions.sh create mode 100644 hooks/libvirt-hook-config.xml create mode 100755 hooks/qemu create mode 100644 hooks/qemu.d/Makefile.am create mode 100755 hooks/qemu.d/dynamic-hugepages.sh diff --git a/Makefile.am b/Makefile.am index 9796069..e4a709a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ examples/dominfo examples/domsuspend examples/apparmor \ examples/xml/nwfilter examples/openauth examples/systemtap \ tools/wireshark examples/dommigrate \ - examples/lxcconvert examples/domtop + examples/lxcconvert examples/domtop hooks ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 93f9e38..b0dc035 100644 --- a/configure.ac +++ b/configure.ac @@ -2812,7 +2812,8 @@ AC_CONFIG_FILES([\ examples/xml/nwfilter/Makefile \ examples/lxcconvert/Makefile \ tools/wireshark/Makefile \ -tools/wireshark/src/Makefile]) +tools/wireshark/src/Makefile \ +hooks/Makefile hooks/daemon.d/Makefile hooks/qemu.d/Makefile]) AC_OUTPUT AC_MSG_NOTICE([]) diff --git a/hooks/Makefile.am b/hooks/Makefile.am new file mode 100644 index 000..1fe6076 --- /dev/null +++ b/hooks/Makefile.am @@ -0,0 +1,27 @@ +## Copyright (C) 2015 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 even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +## Lesser General Public License for more details. +## +## You should have received a copy of the GNU Lesser General Public +## License along with this library. If not, see +## http://www.gnu.org/licenses/. + +SUBDIRS = daemon.d qemu.d + +hooksdir = $(pkgdatadir)/hooks +hooks_DATA = \ + daemon \ + functions.sh \ + libvirt-hook-config.xml \ + qemu \ + README + +EXTRA_DIST =
[libvirt] Jenkins build is back to normal : libvirt-syntax-check #3635
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/3635/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list