Re: [libvirt] [PATCH v4 1/1] migration: add support for migrateURI configuration
On Fri, 2014-05-16 at 09:51 -0400, Daniel P. Berrange wrote: On Thu, May 15, 2014 at 11:50:54AM +0200, Jiri Denemark wrote: On Wed, May 14, 2014 at 15:18:09 +0800, Chen Fan wrote: For now, we set the migration URI via command line '--migrate_uri' or construct the URI by looking up the dest host's hostname which could be solved by DNS automatically. But in cases the dest host have two or more NICs to reach, we may need to send the migration data over a specific NIC which is different from the automatically resloved one for some reason like performance, security, etc. thus we must explicitly specify the migrateuri in command line everytime, but it is too troublesome if there are many such hosts(and don't forget virt-manager). This patches add a configuration file option on dest host to save the default migrate uri which explicitly specify which of this host's addresses is used for transferring data, thus user doesn't boring to specify it in command line everytime. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- v3-v4: move up the default uri_in setting to qemuDomainMigratePrepare3Params() src/qemu/qemu.conf | 6 +- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 2 +- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f0e802f..6b443d0 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -449,7 +449,11 @@ # #seccomp_sandbox = 1 - +# Override the migration URI for specifying one of host's IP addresses +# to transfer the migration data stream. +# Defaults to hostname, both IPv4 and IPv6 addresses are accepted. +# +#migrate_uri = tcp://192.168.0.1 # Override the listen address for all incoming migrations. Defaults to # 0.0.0.0, or :: if both host and qemu are capable of IPv6. The more I think about this the more I incline to a slightly different approach. Rather than providing a way to override migration URI, we could just provide an option in libvirtd.conf to override what virGetHostname returns. That is, the option (naturally called hostname) would tell libvirt to use the configured hostname (which might even be just an IP address) instead of trying to detect it. I'm not sure this is a good idea. The hostname is used in a number of places in libvirt, and we don't neccessarily want them all to use the same interface as the migration data. eg if you have a NIC that is dedicated just for migration traffic, you won't want to force other parts of libvirt to use that. So having the migration hostname specified separately is a good idea IMHO. Perhaps we could simplify though by having 'migrate_host' rather than 'migrate_uri' ? I think that is a good idea. I would like to implement them. Thanks, Chen And perhaps we do want a hostname override globally anyway, for other reasons. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCHv2] Don't log an internal error when the guest hasn't updated balloon stats
On 05/16/2014 03:13 PM, Luiz Capitulino wrote: On Fri, 16 May 2014 00:11:24 -0600 Eric Blake ebl...@redhat.com wrote: Is no stats yet really an error? This is a special case where the guest hasn't ever filled QEMU with balloon stats. There are two possible cases. Either the guest hasn't done it yet, but will do in the future or the guest will never do it (eg. the guest doesn't support balloon, the guest crashed, etc). Libvirt has done nothing wrong, and I'd argue the guest hasn't done anything wrong, either. Should we simply return an empty result? Like cat on a file that hasn't gotten its data, yet. Yes, that would be reasonable. I'm fine with the two possible solutions here: adding a new TryAgain error class or returning an empty result. I say empty because those fields are not optionals, so we'll have to fill them with some value. Shouldn't be a problem for most fields, as the spec (docs/virtio-balloon-stats.txt) already defines that stats that the guest doesn't report are returned as -1. The only exception here is the last-update field, which can't hold a negative iirc. The only choice is to return 0 there. I guess that this shouldn't be a problem either. Who volunteers to fix this? I've tried: http://marc.info/?l=qemu-develm=140048179520115w=2 Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
Hi Cole, This is a patch similar with your previous patch to fix for ARM. Do you have any comments on it? Cindy, Since you are the main contributor to QEMU driver on PPC, I'll also appreciate your comments. Best Regards, Olivia -Original Message- From: Yin Olivia-R63875 Sent: Friday, May 16, 2014 8:38 AM To: Yin Olivia-R63875; libvir-list@redhat.com Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC Ping. This is a patch similar with ARM platforms. http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc968171c5 84ec2edcfdcb8fadde Right now on ppce500, chardev is not supported for the serial console. So it uses the the legacy -serial option. Best Regards, Olivia -Original Message- From: Olivia Yin [mailto:hong-hua@freescale.com] Sent: Wednesday, May 14, 2014 6:47 PM To: libvir-list@redhat.com Cc: Yin Olivia-R63875 Subject: [PATCH] qemu: Fix specifying char devs for PPC QEMU ppce500 board uses the old style -serial options. Other PPC boards don't give any way to explicitly wire in a -chardev except pseries which uses -device spapr-vty with -chardev. --- src/qemu/qemu_capabilities.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64) + (def-os.arch != VIR_ARCH_PPC) (def-os.arch != +VIR_ARCH_PPC64)) return true; /* This may not be true for all ARM machine types, but at least * the only supported non-virtio serial devices of vexpress and versatile - * don't have the -chardev property wired up. */ + * don't have the -chardev property wired up. + * For PPC machines, only pserial need -device spapr-vty with + -chardev */ return (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE - chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); + chr-targetType == + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) || +(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL + chr-info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); } -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] blockcommit: document semantics of committing active layer
On 05/16/14 22:17, Eric Blake wrote:
Now that qemu 2.0 allows commit of the active layer, people are
attempting to use virsh blockcommit and getting into a stuck
state, because libvirt is unprepared to handle the two-phase
commit required by qemu.
Stepping back a bit, there are two valid semantics for a
commit operation:
1. Maintain a 'golden' base, and a transient overlay. Make
changes in the overlay, and if everything appears to work,
commit those changes into the base, but still keep the overlay
for the next round of changes; repeat the cycle as desired.
2. Create an external snapshot, then back up the stable state
in the backing file. Once the backup is complete, commit the
overlay back into the base, and delete the temporary snapshot.
Since qemu doesn't know up front which of the two styles is
preferred, a block commit of the active layer merely gets
the job into a synchronized state, and sends an event; then
the user must either cancel (case 1) or complete (case 2),
where qemu then sends a second event that actually ends the
job. However, libvirt was blindly assuming the semantics that
apply to a commit of an intermediate image, where there is
only one sane conclusion (the job automatically ends with
fewer elements in the chain); and getting stuck because it
wasn't prepared for qemu to enter a second phase of the job.
This patch adds a flag to the libvirt API that a user MUST
supply in order to acknowledge that they will be using two-phase
semantics. It might be possible to have a mode where if the
flag is omitted, we automatically do the case 2 semantics on
the user's behalf; but before that happens, I must do additional
patches to track the fact that we are doing an active commit
in the domain XML. So the safest thing for now is to reject the
new flag in qemu, as well as prevent commits of the active layer
without the flag. Later patches will add support of the flag,
and once 2-phase semantics are working, we can then decide
whether to relax things to allow an omitted flag to cause an
automatic pivot.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)
(VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT): New enums.
* src/libvirt.c (virDomainBlockCommit): Document two-phase job
when committing active layer, through new flag.
(virDomainBlockJobAbort): Document that pivot also occurs after
active commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Explicitly
reject active copy; later patches will add it in.
Signed-off-by: Eric Blake ebl...@redhat.com
---
This patch should probably be backported, even if later patches
in the series are not, so that users avoid getting hung.
include/libvirt/libvirt.h.in | 12 ++
src/libvirt.c| 55
+---
src/qemu/qemu_driver.c | 9
3 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 260c971..127de11 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2513,6 +2513,7 @@ typedef enum {
VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1,
VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2,
VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT = 3,
+VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT = 4,
#ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_BLOCK_JOB_TYPE_LAST
@@ -2523,7 +2524,8 @@ typedef enum {
* virDomainBlockJobAbortFlags:
*
* VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion
- * VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT: Pivot to mirror when ending a copy job
+ * VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT: Pivot to new file when ending a copy or
+ * active commit job
*/
typedef enum {
VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1 0,
@@ -2588,6 +2590,8 @@ typedef enum {
VIR_DOMAIN_BLOCK_COMMIT_DELETE = 1 1, /* Delete any files that are
now
invalid after their contents
have been committed */
+VIR_DOMAIN_BLOCK_COMMIT_ACTIVE = 1 2, /* Allow a two-phase commit
when
+ top is the active layer */
} virDomainBlockCommitFlags;
int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char
*base,
@@ -4823,8 +4827,8 @@ typedef void
(*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
/**
* virConnectDomainEventBlockJobStatus:
*
- * The final status of a virDomainBlockPull() or virDomainBlockRebase()
- * operation
+ * Tracks status of a virDomainBlockPull(), virDomainBlockRebase(),
+ * or virDomainBlockCommit() operation
*/
typedef enum {
VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
@@ -4843,7 +4847,7 @@ typedef enum {
* @dom: domain on which the event occurred
* @disk: fully-qualified filename of the affected disk
* @type: type of block job (virDomainBlockJobType)
-
[libvirt] libvirt build error in conf/domain_conf.c (Was: Re: [Xen-devel] [libvirt test] 26326: regressions - FAIL)
On Sat, 2014-05-17 at 07:20 +0100, xen.org wrote: flight 26326 libvirt real [real] http://www.chiark.greenend.org.uk/~xensrcts/logs/26326/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 4 libvirt-build fail REGR. vs. 26314 build-armhf-libvirt 4 libvirt-build fail REGR. vs. 26314 build-i386-libvirt4 libvirt-build fail REGR. vs. 26314 This is: conf/domain_conf.c: In function 'virDomainDiskSourceParse': conf/domain_conf.c:4992:9: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] conf/domain_conf.c:5015:21: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] conf/domain_conf.c: In function 'virDomainDiskBackingStoreParse': conf/domain_conf.c:5113:5: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] (from http://www.chiark.greenend.org.uk/~xensrcts/logs/26326/build-amd64-libvirt/4.ts-libvirt-build.log ) I think this isn't Xen specific. I had a look at the recent traffic on libvir-list and didn't see anything related so forwarding to libvirt-list and Eric who seems to have touched the file in question in the range. Ian. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 xen-build-check(1) blocked n/a test-armhf-armhf-libvirt 1 xen-build-check(1) blocked n/a test-amd64-i386-libvirt 1 xen-build-check(1) blocked n/a version targeted for testing: libvirt d18aa7041699343d4df01cd9352e24f215b08c21 baseline version: libvirt 5099084eb30c9c5454e79e8eab8bcff038c0f8cd People who touched revisions under test: Chen Hanxiao chenhanx...@cn.fujitsu.com Eric Blake ebl...@redhat.com Jiri Denemark jdene...@redhat.com Michal Privoznik mpriv...@redhat.com jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-oldkern pass build-i386-oldkern pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt blocked sg-report-flight on osstest.cam.xci-test.com logs: /home/xc_osstest/logs images: /home/xc_osstest/images Logs, config files, etc. are available at http://www.chiark.greenend.org.uk/~xensrcts/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Not pushing. commit d18aa7041699343d4df01cd9352e24f215b08c21 Author: Chen Hanxiao chenhanx...@cn.fujitsu.com Date: Tue May 13 16:01:16 2014 +0800 util: fix memory leak in failure path of virCgroupKillRecursiveInternal Don't leak keypath when we fail to kill a process Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com commit b279e52f7bd07dfe6e7f5ef0b34f2b424c50eee2 Author: Eric Blake ebl...@redhat.com Date: Wed May 14 16:40:33 2014 -0600 maint: prefer enum over int for virstoragefile structs For internal structs, we might as well be type-safe and let the compiler help us with less typing required on our part (getting rid of casts is always nice). In trying to use enums directly, I noticed two problems in virstoragefile.h that can't be fixed without more invasive refactoring: virStorageSource.format is used as more of a union of multiple enums in storage volume code (so it has to remain an int), and virStorageSourcePoolDef refers to pooltype whose enum is declared in src/conf, but where src/util can't pull in headers from src/conf. * src/util/virstoragefile.h (virStorageNetHostDef) (virStorageSourcePoolDef, virStorageSource): Use enums instead of int for fields of internal types. * src/qemu/qemu_command.c (qemuParseCommandLine): Cover all values. *
[libvirt] [PATCH] security_dac: Use virDomainTPMBackendType when possible
It's certainly useful to use enum typecast in switch().
Currently, it's not used only in two places:
virSecurityDACSetSecurityTPMFileLabel and
virSecurityDACRestoreSecurityTPMFileLabel.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/security/security_dac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 05303e7..7e0f160 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -847,7 +847,7 @@ virSecurityDACSetSecurityTPMFileLabel(virSecurityManagerPtr
mgr,
{
int ret = 0;
-switch (tpm-type) {
+switch ((enum virDomainTPMBackendType) tpm-type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
ret = virSecurityDACSetChardevLabel(mgr, def, NULL,
tpm-data.passthrough.source);
@@ -867,7 +867,7 @@
virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr mgr,
{
int ret = 0;
-switch (tpm-type) {
+switch ((enum virDomainTPMBackendType) tpm-type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
ret = virSecurityDACRestoreChardevLabel(mgr, def, NULL,
tpm-data.passthrough.source);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virSecurityDACRestoreSecurityHostdevLabel: Unmark @def as unused
The domain definition is clearly used a few lines below so there's no need to mark @def as unused. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/security/security_dac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 289c2f0..05303e7 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -598,7 +598,7 @@ virSecurityDACRestoreSecuritySCSILabel(virSCSIDevicePtr dev ATTRIBUTE_UNUSED, static int virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, virDomainHostdevDefPtr dev, const char *vroot) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virSecurityDACRestoreSecurityHostdevLabel: Unmark @def as unused
On 05/19/2014 11:39 AM, Michal Privoznik wrote: The domain definition is clearly used a few lines below so there's no need to mark @def as unused. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/security/security_dac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK, trivial signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] security_dac: Use virDomainTPMBackendType when possible
On 05/19/2014 11:39 AM, Michal Privoznik wrote: It's certainly useful to use enum typecast in switch(). Currently, it's not used only in two places: virSecurityDACSetSecurityTPMFileLabel and virSecurityDACRestoreSecurityTPMFileLabel. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/security/security_dac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) The tpm type is declared as enum virDomainTPMBackendType, no need to typecast it. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix virObjectUnlock() usage
On Sat, May 17, 2014 at 11:44:48PM +0400, Roman Bogorodskiy wrote: In a number of places in the bhyve driver, virObjectUnlock() is called with an arg without check if the arg is non-NULL, which could result in passing NULL value and a warning like: virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance * src/bhyve/bhyve_driver.c (bhyveDomainGetInfo) (bhyveDomainGetState, bhyveDomainGetAutostart) (bhyveDomainSetAutostart, bhyveDomainIsActive) (bhyveDomainIsPersistent, bhyveDomainGetXMLDesc) (bhyveDomainUndefine, bhyveDomainLookupByUUID) (bhyveDomainLookupByName, bhyveDomainLookupByID) (bhyveDomainCreateWithFlags, bhyveDomainOpenConsole): Check if arg is not NULL before calling virObjectUnlock on it. --- src/bhyve/bhyve_driver.c | 39 ++- 1 file changed, 26 insertions(+), 13 deletions(-) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix crash in DAC driver with no seclabels
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel
dereferences the NULL seclabel when checking if norelabel is set.
Remove this check, since it is already done in RestoreSecurityAllLabel
and if norelabel is set, RestoreChardevLabel is never called.
---
src/security/security_dac.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 05303e7..00f47cb 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -768,22 +768,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
static int
virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainDefPtr def,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainChrDefPtr dev,
virDomainChrSourceDefPtr dev_source)
{
-virSecurityLabelDefPtr seclabel;
virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
char *in = NULL, *out = NULL;
int ret = -1;
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-
if (dev)
chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
SECURITY_DAC_NAME);
-if (seclabel-norelabel || (chr_seclabel chr_seclabel-norelabel))
+if (chr_seclabel chr_seclabel-norelabel)
return 0;
switch ((enum virDomainChrType) dev_source-type) {
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] parallels: add disks correctly
On Thu, May 08, 2014 at 11:37:09AM +0100, Daniel P. Berrange wrote:
On Wed, May 07, 2014 at 10:04:08PM +0400, Dmitry Guryanov wrote:
Disks support in this driver was implemented with an assumption,
that disk images can't be created by hand, without VM. So
complex storage driver was implemented with workaround.
This is not true, we can create new disks using ploop tool.
So the first step to reimplement disks support in parallels
driver is to do not use information from the storage driver,
until we will implement VIR_STORAGE_TYPE_VOLUME disks.
So after this patch disks can be added in the same way as
in any other driver: you create a disk image and then add
an entry to the XML definition of the domain with path to that
image file, for example:
disk type='file' device='disk'
driver type='ploop'/
source file='/storage/harddisk1.hdd'/
target dev='sda' bus='sata'/
address type='drive' controller='0' bus='0' target='0' unit='0'/
/disk
This patch makes parallels storage driver useless, but I'll fix it
later. Now you can create an image by hand, using ploop tool,
and then add it to some domain.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
src/parallels/parallels_driver.c | 83
++--
1 file changed, 28 insertions(+), 55 deletions(-)
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c
index b2de12f..67b28c4 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1605,17 +1605,34 @@ parallelsApplyVideoParams(parallelsDomObjPtr pdom,
return 0;
}
-static int parallelsAddHddByVolume(parallelsDomObjPtr pdom,
- virDomainDiskDefPtr disk,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr voldef)
+static int parallelsAddHdd(parallelsDomObjPtr pdom,
+ virDomainDiskDefPtr disk)
{
int ret = -1;
+const char *src = virDomainDiskGetSource(disk);
+int type = virDomainDiskGetType(disk);
const char *strbus;
virCommandPtr cmd = virCommandNewArgList(PRLCTL, set, pdom-uuid,
--device-add, hdd, NULL);
-virCommandAddArgFormat(cmd, --size=%lluM, voldef-target.capacity
20);
+
+if (type == VIR_STORAGE_TYPE_FILE) {
+int format = virDomainDiskGetFormat(disk);
+
+if (format != VIR_STORAGE_FILE_PLOOP)
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _(Invalid disk format: %d), type);
Missing a 'goto cleanup' after reporting the error, so that
execution returns
Since this was the only bug in the series, I've gone ahead and fixed it
myself and pushed all four patches to git.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v8 0/4] Handling of undefine and re define snapshots with VirtualBox 4.2 or higher
Hello,
This is a new series of patches in order to support undefining and redefining
snapshots with VirtualBox 4.2 or higher.
These patches are based on Manuel Vives' patches, taking into account Daniel
P. Berrange's remarks.
The VirtualBox API provides only high level operations to manipulate snapshots,
so it not possible to support flags like VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE and
VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY with only API calls.
Following an IRC talk with Eric Blake, the decision was made to emulate these
behaviours by manipulating directly the .vbox XML files.
The first patch adds extra details in the snapshot XML returned by libvirt. We
will need those details in order to redefine the snapshots.
The second patch adds a new API to manipulate the VirtualBox XML file. It
provides several structs describing the VirtualBox XML file nodes and
functions which can manipulate these structs.
The third patch adds support of the VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE
and VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flags in virDomainSnapshotCreateXML.
The idea is to unregister the machine, add the snapshot in the Virtualbox XML
file and re-register the machine.
However, VirtualBox forbids a machine to have snapshots but no current
snapshot. So, if the real current snapshot has not been redefined yet, we
create fake disks, allowing us to have an intermediate state in order to not
corrupt the snapshot's read-write disks. These fake disks will be deleted
during the
next redefine.
The fourth and last patch adds support of the
VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY flag in virDomainSnapshotDelete.
As in the third patch, we also create fake disks to not corrupt the snapshot's
read-write disks.
The patches were only tested with VirtualBox 4.3.10 and VirtualBox 4.2.24.
Regards
Yohan BELLEGUIC
v8:
* Fix patches according to Daniel P. Berrange review (May 01, 2014)
* Rename all the methods in vbox_snapshot.{c,h}
* Add a test case for serialize and deserialize a virVBoxSnapshotConfMachine
* Fix memory leaks
v7:
* Add vbox_snapshot_conf.{h,c} files to (de)serialize VirtualBox XML files
* Update the code to use the API exposed by vbox_snapshot_conf.h
* Handle the fact that VirtualBox forbids a machine to have snapshots but no
current snapshot
v6:
* Rebased because of a massive change in vbox_tmpl.c due to changes in
the handling of different versions of VirtualBox
v5:
* The patches are modified according to a first review by Laine Stump:
* renamed virSearchUuid to virSearchRegex and moved it from
viruuid.{c,h} to virstring.{c,h}.
* Various fixes.
v4:
* The code is compliant with Virtualbox 4.3 API
* Some minor modifications in order to satisfy make syntax-check
v3:
* Use of STREQ_NULLABLE instead of STREQ in one case
* Fix the method for finding uuids according to Ján Tomko review
v2:
* Fix a licence problem with the method for string replacement
Manuel VIVES (1):
vbox_tmpl.c: Better XML description for snapshots
Yohan BELLEGUIC (3):
Add vbox_snapshot_conf struct
vbox_tmpl.c: Patch for redefining snapshots
vbox_tmpl.c: Add function for undefining snapshot
po/POTFILES.in |1 +
src/Makefile.am|1 +
src/vbox/vbox_snapshot_conf.c | 1490 +++
src/vbox/vbox_snapshot_conf.h | 105 ++
src/vbox/vbox_tmpl.c | 1982 +++-
tests/Makefile.am | 15 +
tests/vboxsnapshotxmldata/2disks-1snap.vbox| 322
tests/vboxsnapshotxmldata/2disks-2snap.vbox| 478 +
.../vboxsnapshotxmldata/2disks-3snap-brother.vbox | 786
tests/vboxsnapshotxmldata/2disks-3snap.vbox| 636 +++
tests/vboxsnapshotxmldata/2disks-nosnap.vbox | 168 ++
tests/vboxsnapshotxmltest.c| 161 ++
12 files changed, 6100 insertions(+), 45 deletions(-)
create mode 100644 src/vbox/vbox_snapshot_conf.c
create mode 100644 src/vbox/vbox_snapshot_conf.h
create mode 100644 tests/vboxsnapshotxmldata/2disks-1snap.vbox
create mode 100644 tests/vboxsnapshotxmldata/2disks-2snap.vbox
create mode 100644 tests/vboxsnapshotxmldata/2disks-3snap-brother.vbox
create mode 100644 tests/vboxsnapshotxmldata/2disks-3snap.vbox
create mode 100644 tests/vboxsnapshotxmldata/2disks-nosnap.vbox
create mode 100644 tests/vboxsnapshotxmltest.c
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v8 1/4] vbox_tmpl.c: Better XML description for snapshots
From: Manuel VIVES manuel.vi...@diateam.net
It will be needed for the future patches because we will
redefine snapshots
---
src/vbox/vbox_tmpl.c | 476 +-
1 file changed, 471 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index e124e69..5d4a7ba 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -58,6 +58,8 @@
#include fdstream.h
#include viruri.h
#include virstring.h
+#include virtime.h
+#include virutil.h
/* This one changes from version to version. */
#if VBOX_API_VERSION == 2002000
@@ -6039,7 +6041,9 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
if (!(def = virDomainSnapshotDefParseString(xmlDesc, data-caps,
-data-xmlopt, 0, 0)))
+data-xmlopt, -1,
+
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE)))
goto cleanup;
if (def-ndisks) {
@@ -6130,6 +6134,431 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
return ret;
}
+#if VBOX_API_VERSION =4002000
+static
+int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def,
+virDomainSnapshotPtr snapshot)
+{
+virDomainPtr dom = snapshot-domain;
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+vboxIID domiid = VBOX_IID_INITIALIZER;
+IMachine *machine = NULL;
+ISnapshot *snap = NULL;
+IMachine *snapMachine = NULL;
+vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER;
+PRUint32 maxPortPerInst[StorageBus_Floppy + 1] = {};
+PRUint32 maxSlotPerPort[StorageBus_Floppy + 1] = {};
+int diskCount = 0;
+nsresult rc;
+vboxIID snapIid = VBOX_IID_INITIALIZER;
+char *snapshotUuidStr = NULL;
+size_t i = 0;
+
+vboxIIDFromUUID(domiid, dom-uuid);
+rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(no domain with matching UUID));
+goto cleanup;
+}
+if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot-name)))
+goto cleanup;
+
+rc = snap-vtbl-GetId(snap, snapIid.value);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Could not get snapshot id));
+goto cleanup;
+}
+
+VBOX_UTF16_TO_UTF8(snapIid.value, snapshotUuidStr);
+rc = snap-vtbl-GetMachine(snap, snapMachine);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(could not get machine));
+goto cleanup;
+}
+def-ndisks = 0;
+rc = vboxArrayGet(mediumAttachments, snapMachine,
snapMachine-vtbl-GetMediumAttachments);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(no medium attachments));
+goto cleanup;
+}
+/* get the number of attachments */
+for (i = 0; i mediumAttachments.count; i++) {
+IMediumAttachment *imediumattach = mediumAttachments.items[i];
+if (imediumattach) {
+IMedium *medium = NULL;
+
+rc = imediumattach-vtbl-GetMedium(imediumattach, medium);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot get medium));
+goto cleanup;
+}
+if (medium) {
+def-ndisks++;
+VBOX_RELEASE(medium);
+}
+}
+}
+/* Allocate mem, if fails return error */
+if (VIR_ALLOC_N(def-disks, def-ndisks) 0)
+goto cleanup;
+
+if (!vboxGetMaxPortSlotValues(data-vboxObj, maxPortPerInst,
maxSlotPerPort))
+goto cleanup;
+
+/* get the attachment details here */
+for (i = 0; i mediumAttachments.count diskCount def-ndisks; i++) {
+IStorageController *storageController = NULL;
+PRUnichar *storageControllerName = NULL;
+PRUint32 deviceType = DeviceType_Null;
+PRUint32 storageBus = StorageBus_Null;
+IMedium *disk = NULL;
+PRUnichar *childLocUtf16 = NULL;
+char *childLocUtf8 = NULL;
+PRUint32 deviceInst = 0;
+PRInt32devicePort = 0;
+PRInt32deviceSlot = 0;
+vboxArray children = VBOX_ARRAY_INITIALIZER;
+vboxArray snapshotIids = VBOX_ARRAY_INITIALIZER;
+IMediumAttachment *imediumattach = mediumAttachments.items[i];
+size_t j = 0;
+size_t k = 0;
+if (!imediumattach)
+continue;
+rc = imediumattach-vtbl-GetMedium(imediumattach, disk);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+
[libvirt] [PATCH v8 4/4] vbox_tmpl.c: Add function for undefining snapshot
All snapshots information will be deleted from the vbox XML, but
differencing disks will be kept so the user will be able to redefine the
snapshot.
---
src/vbox/vbox_tmpl.c | 464 +-
1 file changed, 463 insertions(+), 1 deletion(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a7f15d4..eb577f4 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -8364,7 +8364,454 @@ vboxDomainSnapshotDeleteTree(vboxGlobalData *data,
return ret;
}
+#if VBOX_API_VERSION = 4002000
+static int
+vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
+{
+/*
+ * This function will remove the node in the vbox xml corresponding to the
snapshot.
+ * It is usually called by vboxDomainSnapshotDelete() with the flag
+ * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY.
+ * If you want to use it anywhere else, be careful, if the snapshot you
want to delete
+ * has children, the result is not granted, they will probably will be
deleted in the
+ * xml, but you may have a problem with hard drives.
+ *
+ * If the snapshot which is being deleted is the current one, we will set
the current
+ * snapshot of the machine to the parent of this snapshot. Before writing
the modified
+ * xml file, we undefine the machine from vbox. After writing the file, we
redefine
+ * the machine with the new file.
+ */
+
+virDomainPtr dom = snapshot-domain;
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+virDomainSnapshotDefPtr def= NULL;
+char *defXml = NULL;
+vboxIID domiid = VBOX_IID_INITIALIZER;
+nsresult rc;
+IMachine *machine = NULL;
+PRUnichar *settingsFilePathUtf16 = NULL;
+char *settingsFilepath = NULL;
+virVBoxSnapshotConfMachinePtr snapshotMachineDesc = NULL;
+int isCurrent = -1;
+int it = 0;
+PRUnichar *machineNameUtf16 = NULL;
+char *machineName = NULL;
+char *nameTmpUse = NULL;
+char *machineLocationPath = NULL;
+PRUint32 aMediaSize = 0;
+IMedium **aMedia = NULL;
+defXml = vboxDomainSnapshotGetXMLDesc(snapshot, 0);
+if (!defXml) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to get XML Desc of snapshot));
+goto cleanup;
+}
+def = virDomainSnapshotDefParseString(defXml,
+ data-caps,
+ data-xmlopt,
+ -1,
+ VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+ VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
+if (!def) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to get a virDomainSnapshotDefPtr));
+goto cleanup;
+}
+
+vboxIIDFromUUID(domiid, dom-uuid);
+rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_NO_DOMAIN, %s,
+ _(no domain with matching UUID));
+goto cleanup;
+}
+rc = machine-vtbl-GetSettingsFilePath(machine, settingsFilePathUtf16);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot get settings file path));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(settingsFilePathUtf16, settingsFilepath);
+
+/*Getting the machine name to retrieve the machine location path.*/
+rc = machine-vtbl-GetName(machine, machineNameUtf16);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot get machine name));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(machineNameUtf16, machineName);
+if (virAsprintf(nameTmpUse, %s.vbox, machineName) 0)
+goto cleanup;
+machineLocationPath = virStringReplace(settingsFilepath, nameTmpUse, );
+if (machineLocationPath == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to get the machine location path));
+goto cleanup;
+}
+snapshotMachineDesc = virVBoxSnapshotConfLoadVboxFile(settingsFilepath,
machineLocationPath);
+if (!snapshotMachineDesc) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot create a vboxSnapshotXmlPtr));
+goto cleanup;
+}
+
+isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc,
def-name);
+if (isCurrent 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to know if the snapshot is the current
snapshot));
+goto cleanup;
+}
+if (isCurrent) {
+/*
+ * If the snapshot is the current snapshot, it means that the machine
has read-write
+ * disks. The first thing to do is to manipulate VirtualBox API to
create
+ * differential read-write disks if the parent snapshot is not null.
+ */
+if (def-parent != NULL) {
+
[libvirt] Wiki account
Hi, I wanted to fix some text at http://wiki.libvirt.org/page/VirtualNetworking#Restricting_virtual_network_traffic_to_a_specific_interface but see that wiki account creation has been disabled. Could I have an account please, thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v8 3/4] vbox_tmpl.c: Patch for redefining snapshots
The machine is unregistered and its vbox XML file is changed in order to
add snapshot information. The machine is then registered with the
snapshot to redefine.
---
src/vbox/vbox_tmpl.c | 976 +-
1 file changed, 970 insertions(+), 6 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 5d4a7ba..a7f15d4 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -43,6 +43,7 @@
#include datatypes.h
#include domain_conf.h
#include snapshot_conf.h
+#include vbox_snapshot_conf.h
#include network_conf.h
#include virerror.h
#include domain_event.h
@@ -6015,6 +6016,958 @@ vboxDomainSnapshotGet(vboxGlobalData *data,
return snapshot;
}
+#if VBOX_API_VERSION = 4002000
+static int vboxCloseDisksRecursively(virDomainPtr dom, char *location)
+{
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+nsresult rc;
+size_t i = 0;
+PRUnichar *locationUtf = NULL;
+IMedium *medium = NULL;
+IMedium **children = NULL;
+PRUint32 childrenSize = 0;
+VBOX_UTF8_TO_UTF16(location, locationUtf);
+rc = data-vboxObj-vtbl-OpenMedium(data-vboxObj,
+ locationUtf,
+ DeviceType_HardDisk,
+ AccessMode_ReadWrite,
+ false,
+ medium);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to open HardDisk, rc=%08x),
+ (unsigned)rc);
+goto cleanup;
+}
+rc = medium-vtbl-GetChildren(medium, childrenSize, children);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s
+ , _(Unable to get disk children));
+goto cleanup;
+}
+for (i = 0; i childrenSize; i++) {
+IMedium *childMedium = children[i];
+if (childMedium) {
+PRUnichar *childLocationUtf = NULL;
+char *childLocation = NULL;
+rc = childMedium-vtbl-GetLocation(childMedium,
childLocationUtf);
+VBOX_UTF16_TO_UTF8(childLocationUtf, childLocation);
+VBOX_UTF16_FREE(childLocationUtf);
+if (vboxCloseDisksRecursively(dom, childLocation) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s
+ , _(Unable to close disk children));
+goto cleanup;
+}
+VIR_FREE(childLocation);
+}
+}
+rc = medium-vtbl-Close(medium);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to close HardDisk, rc=%08x),
+ (unsigned)rc);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VBOX_UTF16_FREE(locationUtf);
+return ret;
+}
+
+static int
+vboxSnapshotRedefine(virDomainPtr dom,
+ virDomainSnapshotDefPtr def,
+ bool isCurrent)
+{
+/*
+ * If your snapshot has a parent,
+ * it will only be redefined if you have already
+ * redefined the parent.
+ *
+ * The general algorithm of this function is below :
+ * First of all, we are going to create our vboxSnapshotXmlMachinePtr
struct from
+ * the machine settings path.
+ * Then, if the machine current snapshot xml file is saved in the machine
location,
+ * it means that this snapshot was previously modified by us and has fake
disks.
+ * Fake disks are added when the flag VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT
was not set
+ * yet, in order to not corrupt read-only disks. The first thing to do is
to remove those
+ * disks and restore the read-write disks, if any, in the
vboxSnapshotXmlMachinePtr struct.
+ * We also delete the current snapshot xml file.
+ *
+ * After that, we are going to register the snapshot read-only disks that
we want to redefine,
+ * if they are not in the media registry struct.
+ *
+ * The next step is to unregister the machine and close all disks.
+ *
+ * Then, we check if the flag VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE has
already been set.
+ * If this flag was set, we just add read-write disks to the media registry
+ * struct. Otherwise, we save the snapshot xml file into the machine
location in order
+ * to recover the read-write disks during the next redefine and we create
differential disks
+ * from the snapshot read-only disks and add them to the media registry
struct.
+ *
+ * Finally, we register the machine with the new virtualbox description
file.
+ */
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+vboxIID domiid = VBOX_IID_INITIALIZER;
+IMachine *machine = NULL;
+nsresult rc;
+PRUnichar *settingsFilePath = NULL;
+char *settingsFilePath_Utf8 = NULL;
+virVBoxSnapshotConfMachinePtr snapshotMachineDesc = NULL;
+char *currentSnapshotXmlFilePath = NULL;
+
Re: [libvirt] [PATCH 3/4] parallels: add disks correctly
On Monday 19 May 2014 13:06:21 Daniel P. Berrange wrote:
On Thu, May 08, 2014 at 11:37:09AM +0100, Daniel P. Berrange wrote:
On Wed, May 07, 2014 at 10:04:08PM +0400, Dmitry Guryanov wrote:
Disks support in this driver was implemented with an assumption,
that disk images can't be created by hand, without VM. So
complex storage driver was implemented with workaround.
This is not true, we can create new disks using ploop tool.
So the first step to reimplement disks support in parallels
driver is to do not use information from the storage driver,
until we will implement VIR_STORAGE_TYPE_VOLUME disks.
So after this patch disks can be added in the same way as
in any other driver: you create a disk image and then add
an entry to the XML definition of the domain with path to that
image file, for example:
disk type='file' device='disk'
driver type='ploop'/
source file='/storage/harddisk1.hdd'/
target dev='sda' bus='sata'/
address type='drive' controller='0' bus='0' target='0' unit='0'/
/disk
This patch makes parallels storage driver useless, but I'll fix it
later. Now you can create an image by hand, using ploop tool,
and then add it to some domain.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
src/parallels/parallels_driver.c | 83
++-- 1 file changed, 28
insertions(+), 55 deletions(-)
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c index b2de12f..67b28c4 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1605,17 +1605,34 @@ parallelsApplyVideoParams(parallelsDomObjPtr
pdom,
return 0;
}
-static int parallelsAddHddByVolume(parallelsDomObjPtr pdom,
- virDomainDiskDefPtr disk,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr voldef)
+static int parallelsAddHdd(parallelsDomObjPtr pdom,
+ virDomainDiskDefPtr disk)
{
int ret = -1;
+const char *src = virDomainDiskGetSource(disk);
+int type = virDomainDiskGetType(disk);
const char *strbus;
virCommandPtr cmd = virCommandNewArgList(PRLCTL, set, pdom-uuid,
--device-add, hdd,
NULL);
-virCommandAddArgFormat(cmd, --size=%lluM, voldef-target.capacity
20); +
+if (type == VIR_STORAGE_TYPE_FILE) {
+int format = virDomainDiskGetFormat(disk);
+
+if (format != VIR_STORAGE_FILE_PLOOP)
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _(Invalid disk format: %d), type);
Missing a 'goto cleanup' after reporting the error, so that
execution returns
Since this was the only bug in the series, I've gone ahead and fixed it
myself and pushed all four patches to git.
Thanks, Daniel, I thought you have objections against first patch, so was
waiting for your answer :)
Regards,
Daniel
--
Dmitry Guryanov
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Wiki account
On Sat, May 17, 2014 at 01:34:57PM +0100, Jonathan Wakely wrote: Hi, I wanted to fix some text at http://wiki.libvirt.org/page/VirtualNetworking#Restricting_virtual_network_traffic_to_a_specific_interface but see that wiki account creation has been disabled. Could I have an account please, thanks. You should have received an email notification with your new account details Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix crash in DAC driver with no seclabels
On 05/19/2014 02:59 PM, Ján Tomko wrote:
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel
dereferences the NULL seclabel when checking if norelabel is set.
Uh, ACK :-) (Since this patch allows a newly rebuilt libvirtd to once
again startup without an immediate crash)
Remove this check, since it is already done in RestoreSecurityAllLabel
and if norelabel is set, RestoreChardevLabel is never called.
---
src/security/security_dac.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 05303e7..00f47cb 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -768,22 +768,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
static int
virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainDefPtr def,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainChrDefPtr dev,
virDomainChrSourceDefPtr dev_source)
{
-virSecurityLabelDefPtr seclabel;
virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
char *in = NULL, *out = NULL;
int ret = -1;
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-
if (dev)
chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
SECURITY_DAC_NAME);
-if (seclabel-norelabel || (chr_seclabel chr_seclabel-norelabel))
+if (chr_seclabel chr_seclabel-norelabel)
return 0;
switch ((enum virDomainChrType) dev_source-type) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] PCI: Introduce new device binding path using pci_dev.driver_override
On 09.05.14 18:50, Alex Williamson wrote:
The driver_override field allows us to specify the driver for a device
rather than relying on the driver to provide a positive match of the
device. This shortcuts the existing process of looking up the vendor
and device ID, adding them to the driver new_id, binding the device,
then removing the ID, but it also provides a couple advantages.
First, the above existing process allows the driver to bind to any
device matching the new_id for the window where it's enabled. This is
often not desired, such as the case of trying to bind a single device
to a meta driver like pci-stub or vfio-pci. Using driver_override we
can do this deterministically using:
echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override
echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind
echo :03:00.0 /sys/bus/pci/drivers_probe
Previously we could not invoke drivers_probe after adding a device
to new_id for a driver as we get non-deterministic behavior whether
the driver we intend or the standard driver will claim the device.
Now it becomes a deterministic process, only the driver matching
driver_override will probe the device.
To return the device to the standard driver, we simply clear the
driver_override and reprobe the device:
echo /sys/bus/pci/devices/:03:00.0/driver_override
echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind
echo :03:00.0 /sys/bus/pci/drivers_probe
Another advantage to this approach is that we can specify a driver
override to force a specific binding or prevent any binding. For
instance when an IOMMU group is exposed to userspace through VFIO
we require that all devices within that group are owned by VFIO.
However, devices can be hot-added into an IOMMU group, in which case
we want to prevent the device from binding to any driver (override
driver = none) or perhaps have it automatically bind to vfio-pci.
With driver_override it's a simple matter for this field to be set
internally when the device is first discovered to prevent driver
matches.
Signed-off-by: Alex Williamson alex.william...@redhat.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
v2: Use strchr() as suggested by Guenter Roeck and adopted by the
platform driver version of this same interface.
Documentation/ABI/testing/sysfs-bus-pci | 21
drivers/pci/pci-driver.c| 25 +--
drivers/pci/pci-sysfs.c | 40 +++
include/linux/pci.h |1 +
4 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-pci
b/Documentation/ABI/testing/sysfs-bus-pci
index a3c5a66..898ddc4 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -250,3 +250,24 @@ Description:
valid. For example, writing a 2 to this file when sriov_numvfs
is not 0 and not 2 already will return an error. Writing a 10
when the value of sriov_totalvfs is 8 will return an error.
+
+What: /sys/bus/pci/devices/.../driver_override
+Date: April 2014
+Contact: Alex Williamson alex.william...@redhat.com
+Description:
+ This file allows the driver for a device to be specified which
+ will override standard static and dynamic ID matching. When
+ specified, only a driver with a name matching the value written
+ to driver_override will have an opportunity to bind to the
+ device. The override is specified by writing a string to the
+ driver_override file (echo pci-stub driver_override) and
+ may be cleared with an empty string (echo driver_override).
+ This returns the device to standard matching rules binding.
+ Writing to driver_override does not automatically unbind the
+ device from its current driver or make any attempt to
+ automatically load the specified driver. If no driver with a
+ matching name is currently loaded in the kernel, the device
+ will not bind to any driver. This also allows devices to
+ opt-out of driver binding using a driver_override name such as
+ none. Only a single driver may be specified in the override,
+ there is no support for parsing delimiters.
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index d911e0c..4393c12 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -216,6 +216,13 @@ const struct pci_device_id *pci_match_id(const struct
pci_device_id *ids,
return NULL;
}
+static const struct pci_device_id pci_device_id_any = {
+ .vendor = PCI_ANY_ID,
+ .device = PCI_ANY_ID,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+};
+
/**
* pci_match_device - Tell if a PCI
Re: [libvirt] [PATCH] Fix crash in DAC driver with no seclabels
On 05/19/2014 03:20 PM, Laine Stump wrote: On 05/19/2014 02:59 PM, Ján Tomko wrote: With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel dereferences the NULL seclabel when checking if norelabel is set. Uh, ACK :-) (Since this patch allows a newly rebuilt libvirtd to once again startup without an immediate crash) Thanks, pushed now. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
On 05/19/2014 03:41 AM, hong-hua@freescale.com wrote: Hi Cole, This is a patch similar with your previous patch to fix for ARM. Do you have any comments on it? Cindy, Since you are the main contributor to QEMU driver on PPC, I'll also appreciate your comments. Best Regards, Olivia Patch looks fine, but it should add a qemuxml2argv test case to validate that it actually works. My original patch added test cases later in the patch series, see 54a77c6df3c483864463f602c4c6f435d50bd79e - Cole -Original Message- From: Yin Olivia-R63875 Sent: Friday, May 16, 2014 8:38 AM To: Yin Olivia-R63875; libvir-list@redhat.com Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC Ping. This is a patch similar with ARM platforms. http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc968171c5 84ec2edcfdcb8fadde Right now on ppce500, chardev is not supported for the serial console. So it uses the the legacy -serial option. Best Regards, Olivia -Original Message- From: Olivia Yin [mailto:hong-hua@freescale.com] Sent: Wednesday, May 14, 2014 6:47 PM To: libvir-list@redhat.com Cc: Yin Olivia-R63875 Subject: [PATCH] qemu: Fix specifying char devs for PPC QEMU ppce500 board uses the old style -serial options. Other PPC boards don't give any way to explicitly wire in a -chardev except pseries which uses -device spapr-vty with -chardev. --- src/qemu/qemu_capabilities.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64) + (def-os.arch != VIR_ARCH_PPC) (def-os.arch != +VIR_ARCH_PPC64)) return true; /* This may not be true for all ARM machine types, but at least * the only supported non-virtio serial devices of vexpress and versatile - * don't have the -chardev property wired up. */ + * don't have the -chardev property wired up. + * For PPC machines, only pserial need -device spapr-vty with + -chardev */ return (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE - chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); + chr-targetType == + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) || +(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL + chr-info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); } -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] libxl: Fix up VRAM to minimum requirements
This is a bit debatable. On one side it hides configuration errors
in a way that makes them hard to spot. On the other side there is
at least one issue with (maybe some older versions) virt-manager.
Virt-manager sets VRAM directly, not using the default memory setting
but uses too small values for libxl. Worse, those versions do not seem
to allow to change VRAM from the GUI. So switching the video type to
VGA makes the guest fail to start until one manually adapts the VRAM
size in the XML definition.
With this change this would not happen but VRAM will be bigger than
the GUI says. This would not be that different from current Cirrus
behaviour. Only that in that case qemu seems to ignore the provided
size.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/libxl/libxl_conf.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 2b5c469..9af8abe 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1316,13 +1316,38 @@ libxlSetBuildGraphics(virDomainDefPtr def,
libxl_domain_config *d_config)
* type xen and vga is mapped to cirrus.
*/
if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
switch (def-videos[0]-type) {
case VIR_DOMAIN_VIDEO_TYPE_VGA:
case VIR_DOMAIN_VIDEO_TYPE_XEN:
b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
+ */
+switch (b_info-device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+min_vram = 8 * 1024;
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+default:
+min_vram = 16 * 1024;
+}
break;
case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+switch (b_info-device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+min_vram = 4 * 1024; /* Actually the max, too */
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+default:
+min_vram = 8 * 1024;
+}
break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -1330,7 +1355,7 @@ libxlSetBuildGraphics(virDomainDefPtr def,
libxl_domain_config *d_config)
_(video type not supported by libxl));
return -1;
}
-b_info-video_memkb = def-videos[0]-vram ?
+b_info-video_memkb = (def-videos[0]-vram = min_vram) ?
def-videos[0]-vram :
LIBXL_MEMKB_DEFAULT;
} else {
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] libxl: Implement basic video device selection
This started as an investigation into an issue where libvirt (using the
libxl driver) and the Xen host, like an old couple, could not agree on
who is responsible for selecting the VNC port to use.
Things usually (and a bit surprisingly) did work because, just like that
old couple, they had the same idea on what to do by default. However it
was possible that this ended up in a big argument.
The problem is that display information exists in two different places:
in the vfbs list and in the build info. And for launching the device model,
only the latter is used. But that never gets initialized from libvirt. So
Xen allows the device model to select a default port while libvirt thinks
it has told Xen that this is done by libvirt (though the vfbs config).
While fixing that, I made a stab at actually evaluating the configuration
of the video device. So that it is now possible to at least decide between
a Cirrus or standard VGA emulation and to modify the VRAM within certain
limits using libvirt.
[v2: Check return code of VIR_STRDUP and fix indentation]
[v3: Split out VRAM fixup and return error for unsupported video type]
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/libxl/libxl_conf.c | 68
1 file changed, 68 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b7fed7f..2b5c469 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1304,6 +1304,64 @@ libxlMakeCapabilities(libxl_ctx *ctx)
return NULL;
}
+static int
+libxlSetBuildGraphics(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+break;
+case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s,
+ _(video type not supported by libxl));
+return -1;
+}
+b_info-video_memkb = def-videos[0]-vram ?
+ def-videos[0]-vram :
+ LIBXL_MEMKB_DEFAULT;
+} else {
+libxl_defbool_set(b_info-u.hvm.nographic, 1);
+}
+
+/*
+ * When making the list of displays, only VNC and SDL types were
+ * taken into account. So it seems sensible to connect the default
+ * video device to the first in the vfb list.
+ */
+if (d_config-num_vfbs) {
+libxl_device_vfb *vfb0 = d_config-vfbs[0];
+
+b_info-u.hvm.vnc = vfb0-vnc;
+if (VIR_STRDUP(b_info-u.hvm.vnc.listen, vfb0-vnc.listen) 0)
+return -1;
+if (VIR_STRDUP(b_info-u.hvm.vnc.passwd, vfb0-vnc.passwd) 0)
+return -1;
+b_info-u.hvm.sdl = vfb0-sdl;
+if (VIR_STRDUP(b_info-u.hvm.sdl.display, vfb0-sdl.display) 0)
+return -1;
+if (VIR_STRDUP(b_info-u.hvm.sdl.xauthority, vfb0-sdl.xauthority) 0)
+return -1;
+if (VIR_STRDUP(b_info-u.hvm.keymap, vfb0-keymap) 0)
+return -1;
+}
+
+return 0;
+}
+
int
libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainObjPtr vm, libxl_domain_config *d_config)
@@ -1331,6 +1389,16 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
if (libxlMakePCIList(def, d_config) 0)
return -1;
+/*
+ * Now that any potential VFBs are defined, it is time to update the
+ * build info with the data of the primary display. Some day libxl
+ * might implicitely do so but as it does not right now, better be
+ * explicit.
+ */
+if (d_config-c_info.type == LIBXL_DOMAIN_TYPE_HVM)
+if (libxlSetBuildGraphics(def, d_config) 0)
+return -1;
+
d_config-on_reboot = def-onReboot;
d_config-on_poweroff = def-onPoweroff;
d_config-on_crash = def-onCrash;
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libxl: Enable video device selection for Xen
Sorry, this fell complete off my todos for a while. So I split off the fixup of VRAM into a separate patch which may or may not be used and only accept vga, xen and cirrus as supported types in the main patch. I believe I saw some discussions about how to fix some of the VRAM values as they are passed into qemu. At least for the Cirrus type I saw that the command line looked ok but the guest was getting a much larger VRAM size than it was told. -Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Return error when updating cdrom device
The commit 84c59ffa improved the way we change ejectable media. If for any reason the first eject didn't open the tray we should return with error. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_hotplug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 125a2db..47ec779 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -125,6 +125,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, /* If ret == -1, EjectMedia already set an error message */ virReportError(VIR_ERR_OPERATION_FAILED, %s, _(Unable to eject media)); +ret = -1; } goto audit; } -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-python][PATCH] Implement virDomain{Get, Set}Time APIs
While the setter can be generated automatically, the getter is not.
However, it would be a lot easier if they both share the same logic:
a python dictionary to represent the time: dict['seconds'] to
represent seconds, and dict['nseconds'] to represent microseconds.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
generator.py | 2 +
libvirt-override-virDomain.py | 13 ++
libvirt-override.c| 95 +++
sanitytest.py | 2 +-
4 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/generator.py b/generator.py
index e7b4643..bdac877 100755
--- a/generator.py
+++ b/generator.py
@@ -519,6 +519,8 @@ skip_function = (
'virDomainFSFreeze', # overridden in virDomain.py
'virDomainFSThaw', # overridden in virDomain.py
+'virDomainGetTime', # overridden in virDomain.py
+'virDomainSetTime', # overridden in virDomain.py
# 'Ref' functions have no use for bindings users.
virConnectRef,
diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py
index e61ad00..a50ec0d 100644
--- a/libvirt-override-virDomain.py
+++ b/libvirt-override-virDomain.py
@@ -59,3 +59,16 @@
ret = libvirtmod.virDomainFSThaw(self._o, mountpoints, flags)
if ret == -1: raise libvirtError ('virDomainFSThaw() failed', dom=self)
return ret
+
+def getTime(self, flags=0):
+Extract information about guest time
+ret = libvirtmod.virDomainGetTime(self._o, flags)
+if ret == -1: raise libvirtError ('virDomainGetTime() failed',
dom=self)
+return ret
+
+def setTime(self, time=None, flags=0):
+Set guest time to the given value. @time is a dict conatining
+'seconds' field for seconds and 'nseconds' field for nanosecons
+ret = libvirtmod.virDomainSetTime(self._o, time, flags)
+if ret == -1: raise libvirtError ('virDomainSetTime() failed',
dom=self)
+return ret
diff --git a/libvirt-override.c b/libvirt-override.c
index c4ac223..a7a6213 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7645,6 +7645,99 @@ libvirt_virDomainFSThaw(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args) {
VIR_FREE(mountpoints);
return py_retval;
}
+
+static PyObject *
+libvirt_virDomainGetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
+PyObject *py_retval = NULL;
+PyObject *dict = NULL;
+PyObject *pyobj_domain, *pyobj_seconds, *pyobj_nseconds;
+virDomainPtr domain;
+long long seconds;
+unsigned int nseconds;
+unsigned int flags;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char*)Oi:virDomainGetTime,
+ pyobj_domain, flags))
+return NULL;
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+if (!(dict = PyDict_New()))
+goto cleanup;
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virDomainGetTime(domain, seconds, nseconds, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (c_retval 0)
+goto cleanup;
+
+if (!(pyobj_seconds = libvirt_longlongWrap(seconds)) ||
+PyDict_SetItemString(dict, seconds, pyobj_seconds) 0)
+goto cleanup;
+Py_DECREF(pyobj_seconds);
+
+if (!(pyobj_nseconds = libvirt_uintWrap(nseconds)) ||
+PyDict_SetItemString(dict, nseconds, pyobj_nseconds) 0)
+goto cleanup;
+Py_DECREF(pyobj_nseconds);
+
+py_retval = dict;
+dict = NULL;
+ cleanup:
+Py_XDECREF(dict);
+return py_retval;
+}
+
+
+static PyObject *
+libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
+PyObject *py_retval = NULL;
+PyObject *pyobj_domain;
+PyObject *py_dict;
+virDomainPtr domain;
+long long seconds = 0;
+unsigned int nseconds = 0;
+unsigned int flags;
+ssize_t py_dict_size;
+int c_retval;
+
+if (!PyArg_ParseTuple(args, (char*)OOi:virDomainSetTime,
+ pyobj_domain, py_dict, flags))
+return NULL;
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+py_dict_size = PyDict_Size(py_dict);
+
+if (py_dict_size == 2) {
+PyObject *pyobj_seconds, *pyobj_nseconds;
+
+if (!(pyobj_seconds = PyDict_GetItemString(py_dict, seconds)) ||
+(libvirt_longlongUnwrap(pyobj_seconds, seconds) 0)) {
+PyErr_Format(PyExc_LookupError, malformed or missing 'seconds');
+goto cleanup;
+}
+
+if (!(pyobj_nseconds = PyDict_GetItemString(py_dict, nseconds)) ||
+(libvirt_uintUnwrap(pyobj_nseconds, nseconds) 0)) {
+PyErr_Format(PyExc_LookupError, malformed or missing 'nseconds');
+goto cleanup;
+}
+} else if (py_dict_size 0) {
+PyErr_Format(PyExc_LookupError, Dictionary must contain
+ 'seconds' and 'nseconds');
+goto cleanup;
+}
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+c_retval = virDomainSetTime(domain, seconds, nseconds, flags);
+
[libvirt] [PATCH] avoid 'sync' as variable name
Old gcc complains about shadowing 'sync' variable:
../../src/qemu/qemu_agent.c: In function 'qemuAgentSetTime':
../../src/qemu/qemu_agent.c:1737: warning: declaration of 'sync'
shadows a global declaration [-Wshadow]
/usr/include/unistd.h:464: warning: shadowed declaration is here
[-Wshadow]
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_agent.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 10e2b8d..0421733 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1734,13 +1734,13 @@ int
qemuAgentSetTime(qemuAgentPtr mon,
long long seconds,
unsigned int nseconds,
-bool sync)
+bool rtcSync)
{
int ret = -1;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
-if (sync) {
+if (rtcSync) {
cmd = qemuAgentMakeCommand(guest-set-time, NULL);
} else {
/* guest agent expect time with nanosecond granularity.
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] libvirt build error in conf/domain_conf.c (Was: Re: [libvirt test] 26326: regressions - FAIL)
Ian Campbell wrote: On Sat, 2014-05-17 at 07:20 +0100, xen.org wrote: flight 26326 libvirt real [real] http://www.chiark.greenend.org.uk/~xensrcts/logs/26326/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 4 libvirt-build fail REGR. vs. 26314 build-armhf-libvirt 4 libvirt-build fail REGR. vs. 26314 build-i386-libvirt4 libvirt-build fail REGR. vs. 26314 This is: conf/domain_conf.c: In function 'virDomainDiskSourceParse': conf/domain_conf.c:4992:9: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] conf/domain_conf.c:5015:21: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] conf/domain_conf.c: In function 'virDomainDiskBackingStoreParse': conf/domain_conf.c:5113:5: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] (from http://www.chiark.greenend.org.uk/~xensrcts/logs/26326/build-amd64-libvirt/4.ts-libvirt-build.log ) I think this isn't Xen specific. No, it is compiler specific, and started with http://libvirt.org/git/?p=libvirt.git;a=commit;h=b279e52f7bd07dfe6e7f5ef0b34f2b424c50eee2 Was discussed on #virt last Friday. Eric has a fix under review https://www.redhat.com/archives/libvir-list/2014-May/msg00567.html Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] blockcommit: document semantics of committing active layer
On 05/19/2014 01:58 AM, Peter Krempa wrote:
[Sorry for not putting RFC in the subject line]
+++ b/src/qemu/qemu_driver.c
@@ -15377,6 +15377,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
const char *top_parent = NULL;
bool clean_access = false;
+/* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
@@ -15423,6 +15424,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
top_parent)))
goto endjob;
+/* XXX Should we auto-pivot when COMMIT_ACTIVE is not specified? */
I think we should. But I agree in postponing it to later patch.
+if (topSource == disk-src !(flags VIR_DOMAIN_BLOCK_COPY_ACTIVE))
{
As mentioned in the previous mail, this breaks build.
s/BLOCK_COPY/BLOCK_COMMIT/
Blah - serves me right for sending a patch at the end of my day. But I'm
glad you were able to see the intent.
Also shouldn't this hunk actually go in the patch that adds the active
block commit feature? It seems a bit out of place here.
_This_ patch is fixing the qemu driver to not hang, by acknowledging
that we _don't_ support active block commit (the virCheckFlags() at the
beginning of the function fails if the new flag is specified, and this
check fails if the flag is not specified - which means you cannot do
active commits). The next few patches (when I develop it into a full
series) will first be to add support for the new flag (fixing the first
XXX) and then to relax things to auto-pivot when the flag is not present
(fixing the second XXX).
+virReportError(VIR_ERR_INVALID_ARG,
+ _(commit of '%s' active layer requires active
flag),
+ disk-dst);
+goto endjob;
+}
+
if (!topSource-backingStore) {
virReportError(VIR_ERR_INVALID_ARG,
_(top '%s' in chain for '%s' has no backing file),
I think that this patch should also export this new flag in virsh as we
usually do with API flag additions.
Yes, that's my next task before turning this from an RFC into an actual
patch series, even before the qemu implementation is done.
Additionally a change in virsh is missing again breaks the build process:
Yep, my fault for not actually compile-testing in my rush to get it out
before the weekend :)
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] avoid 'sync' as variable name
On 05/19/2014 08:44 AM, Pavel Hrdina wrote: Old gcc complains about shadowing 'sync' variable: ../../src/qemu/qemu_agent.c: In function 'qemuAgentSetTime': ../../src/qemu/qemu_agent.c:1737: warning: declaration of 'sync' shadows a global declaration [-Wshadow] /usr/include/unistd.h:464: warning: shadowed declaration is here [-Wshadow] Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_agent.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK; qualifies as a trivial build-breaker. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] libvirt build error in conf/domain_conf.c (Was: Re: [libvirt test] 26326: regressions - FAIL)
On 05/19/2014 09:07 AM, Jim Fehlig wrote: Ian Campbell wrote: conf/domain_conf.c:4992:9: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] conf/domain_conf.c:5015:21: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] conf/domain_conf.c: In function 'virDomainDiskBackingStoreParse': conf/domain_conf.c:5113:5: error: comparison of unsigned expression 0 is always false [-Werror=type-limits] (from http://www.chiark.greenend.org.uk/~xensrcts/logs/26326/build-amd64-libvirt/4.ts-libvirt-build.log ) I think this isn't Xen specific. No, it is compiler specific, and started with http://libvirt.org/git/?p=libvirt.git;a=commit;h=b279e52f7bd07dfe6e7f5ef0b34f2b424c50eee2 Was discussed on #virt last Friday. Eric has a fix under review https://www.redhat.com/archives/libvir-list/2014-May/msg00567.html And now committed. Sorry for the extended breakage. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix crash in DAC driver with no seclabels
Ján Tomko wrote:
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel
dereferences the NULL seclabel when checking if norelabel is set.
Yikes!
Remove this check, since it is already done in RestoreSecurityAllLabel
and if norelabel is set, RestoreChardevLabel is never called.
I missed removing this after doing the same analysis for
virSecurityDACSetChardevLabel. Sorry for crashing everyone's libvirtds :-(.
Regards,
Jim
---
src/security/security_dac.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 05303e7..00f47cb 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -768,22 +768,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
static int
virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
- virDomainDefPtr def,
+ virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainChrDefPtr dev,
virDomainChrSourceDefPtr dev_source)
{
-virSecurityLabelDefPtr seclabel;
virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
char *in = NULL, *out = NULL;
int ret = -1;
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-
if (dev)
chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
SECURITY_DAC_NAME);
-if (seclabel-norelabel || (chr_seclabel chr_seclabel-norelabel))
+if (chr_seclabel chr_seclabel-norelabel)
return 0;
switch ((enum virDomainChrType) dev_source-type) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Return error when updating cdrom device
On 05/19/2014 08:07 AM, Pavel Hrdina wrote: The commit 84c59ffa improved the way we change ejectable media. If for any reason the first eject didn't open the tray we should return with error. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_hotplug.c | 1 + 1 file changed, 1 insertion(+) ACK diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 125a2db..47ec779 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -125,6 +125,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, /* If ret == -1, EjectMedia already set an error message */ virReportError(VIR_ERR_OPERATION_FAILED, %s, _(Unable to eject media)); +ret = -1; } goto audit; } -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Return error when updating cdrom device
On 05/19/14 16:07, Pavel Hrdina wrote:
The commit 84c59ffa improved the way we change ejectable media.
If for any reason the first eject didn't open the tray we
should return with error.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_hotplug.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 125a2db..47ec779 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -125,6 +125,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr
driver,
/* If ret == -1, EjectMedia already set an error message */
virReportError(VIR_ERR_OPERATION_FAILED, %s,
_(Unable to eject media));
+ret = -1;
Although this is technically correct, the control flow in this function
seems a little bit weird. The eject media error should be checked before
the polling loop and removed from here. Then also the ret=0 asignment a
few lines below isn't useful.
}
goto audit;
}
I'd go with the following patch:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 125a2db..76e289b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -106,6 +106,9 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr
driver,
ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force);
qemuDomainObjExitMonitor(driver, vm);
+if (ret 0)
+goto audit;
+
virObjectRef(vm);
/* we don't want to report errors from media tray_open polling */
while (retries) {
@@ -121,14 +124,11 @@ int
qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virObjectUnref(vm);
if (retries = 0) {
-if (ret == 0) {
-/* If ret == -1, EjectMedia already set an error message */
-virReportError(VIR_ERR_OPERATION_FAILED, %s,
- _(Unable to eject media));
-}
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+ _(Unable to eject media));
+ret = -1;
goto audit;
}
-ret = 0;
src = virDomainDiskGetSource(disk);
if (src) {
ACK if you go with the above.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix virObjectUnlock() usage
On 05/17/2014 01:44 PM, Roman Bogorodskiy wrote: In a number of places in the bhyve driver, virObjectUnlock() is called with an arg without check if the arg is non-NULL, which could result in passing NULL value and a warning like: virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance Doesn't this instead argue that we should fix virObjectUnlock to gracefully handle a NULL parameter, rather than making every caller uglier? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix virObjectUnlock() usage
Eric Blake wrote: On 05/17/2014 01:44 PM, Roman Bogorodskiy wrote: In a number of places in the bhyve driver, virObjectUnlock() is called with an arg without check if the arg is non-NULL, which could result in passing NULL value and a warning like: virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance Doesn't this instead argue that we should fix virObjectUnlock to gracefully handle a NULL parameter, rather than making every caller uglier? Calling it with NULL seems to be harmless anyway and the only problem is this annoying warning. So, what you propose is checking explicitly for NULL in virObjectUnlock before we do virObjectIsClass(), and if the passed argument is NULL indeed, just return, without logging anything about that? BTW, that's going to be a vast change, quick grep shows more than 750 calls of that function. Roman Bogorodskiy pgpB4h597D4NA.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] avoid 'sync' as variable name
On 19.5.2014 17:16, Eric Blake wrote: On 05/19/2014 08:44 AM, Pavel Hrdina wrote: Old gcc complains about shadowing 'sync' variable: ../../src/qemu/qemu_agent.c: In function 'qemuAgentSetTime': ../../src/qemu/qemu_agent.c:1737: warning: declaration of 'sync' shadows a global declaration [-Wshadow] /usr/include/unistd.h:464: warning: shadowed declaration is here [-Wshadow] Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_agent.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK; qualifies as a trivial build-breaker. Thanks, pushed, Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Return error when updating cdrom device
On 19.5.2014 18:40, Peter Krempa wrote:
On 05/19/14 16:07, Pavel Hrdina wrote:
The commit 84c59ffa improved the way we change ejectable media.
If for any reason the first eject didn't open the tray we
should return with error.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_hotplug.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 125a2db..47ec779 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -125,6 +125,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr
driver,
/* If ret == -1, EjectMedia already set an error message */
virReportError(VIR_ERR_OPERATION_FAILED, %s,
_(Unable to eject media));
+ret = -1;
Although this is technically correct, the control flow in this function
seems a little bit weird. The eject media error should be checked before
the polling loop and removed from here. Then also the ret=0 asignment a
few lines below isn't useful.
}
goto audit;
}
I'd go with the following patch:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 125a2db..76e289b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -106,6 +106,9 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr
driver,
ret = qemuMonitorEjectMedia(priv-mon, driveAlias, force);
qemuDomainObjExitMonitor(driver, vm);
+if (ret 0)
+goto audit;
+
virObjectRef(vm);
/* we don't want to report errors from media tray_open polling */
while (retries) {
@@ -121,14 +124,11 @@ int
qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virObjectUnref(vm);
if (retries = 0) {
-if (ret == 0) {
-/* If ret == -1, EjectMedia already set an error message */
-virReportError(VIR_ERR_OPERATION_FAILED, %s,
- _(Unable to eject media));
-}
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+ _(Unable to eject media));
+ret = -1;
goto audit;
}
-ret = 0;
src = virDomainDiskGetSource(disk);
if (src) {
ACK if you go with the above.
Peter
Thanks, pushed with the change above,
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix virObjectUnlock() usage
On 05/19/2014 10:57 AM, Roman Bogorodskiy wrote: Eric Blake wrote: On 05/17/2014 01:44 PM, Roman Bogorodskiy wrote: In a number of places in the bhyve driver, virObjectUnlock() is called with an arg without check if the arg is non-NULL, which could result in passing NULL value and a warning like: virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance Doesn't this instead argue that we should fix virObjectUnlock to gracefully handle a NULL parameter, rather than making every caller uglier? Calling it with NULL seems to be harmless anyway and the only problem is this annoying warning. So, what you propose is checking explicitly for NULL in virObjectUnlock before we do virObjectIsClass(), and if the passed argument is NULL indeed, just return, without logging anything about that? Yes, since we have other virObject code that special cases NULL (for example, virObjectUnref). BTW, that's going to be a vast change, quick grep shows more than 750 calls of that function. It doesn't invalidate any existing caller to make virObjectUnlock() special-case NULL. And while it DOES make any existing caller that also checks for NULL be a case of redundant code, we don't have to clean all callers up in a single patch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix virObjectUnlock() usage
Eric Blake wrote: BTW, that's going to be a vast change, quick grep shows more than 750 calls of that function. It doesn't invalidate any existing caller to make virObjectUnlock() special-case NULL. And while it DOES make any existing caller that also checks for NULL be a case of redundant code, we don't have to clean all callers up in a single patch. Good, so I'll add a check for NULL in virObjectUnlock() and prepare a series that cleans up the redundant if (obj) check before calling virObjectUnlock(obj), say, on per-driver basis. Roman Bogorodskiy pgpPGrb6NFUPe.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] Add explicit non-NULL check for virObjectUnlock()
A lot of users of virObjectUnlock() use it this way:
if (obj)
virObjectUnlock(obj);
And while passing NULL is not harmless, it generates a warning like
virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance
To make this function usage simplier, check if argument is not NULL
before calling virObjectIsClass() and allow caller not to care about
that.
---
src/util/virobject.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 6cb84b4..19e1268 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -335,6 +335,9 @@ void virObjectUnlock(void *anyobj)
{
virObjectLockablePtr obj = anyobj;
+if (!obj)
+return;
+
if (!virObjectIsClass(obj, virObjectLockableClass)) {
VIR_WARN(Object %p (%s) is not a virObjectLockable instance,
obj, obj ? obj-parent.klass-name : (unknown));
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] libxl: virObjectUnlock usage cleanup
--- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 124 --- 2 files changed, 42 insertions(+), 85 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 80d5280..efcdca7 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -602,8 +602,7 @@ libxlDomainShutdownThread(void *opaque) libxlDomainStart(driver, vm, 0, -1); cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (dom_event) libxlDomainEventQueue(driver, dom_event); libxl_event_free(ctx, ev); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index df7d510..3792515 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -664,8 +664,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, cleanup: virDomainDefFree(def); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); virObjectUnref(cfg); return dom; } @@ -691,8 +690,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -717,8 +715,7 @@ libxlDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -743,8 +740,7 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -798,8 +794,7 @@ libxlDomainSuspend(virDomainPtr dom) vm = NULL; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -858,8 +853,7 @@ libxlDomainResume(virDomainPtr dom) vm = NULL; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -920,8 +914,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) } cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -968,8 +961,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) } cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1017,7 +1009,6 @@ libxlDomainDestroyFlags(virDomainPtr dom, ret = 0; cleanup: -if (vm) virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); @@ -1046,8 +1037,7 @@ libxlDomainGetOSType(virDomainPtr dom) goto cleanup; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return type; } @@ -1066,8 +1056,7 @@ libxlDomainGetMaxMemory(virDomainPtr dom) ret = vm-def-mem.max_balloon; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1193,8 +1182,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, vm = NULL; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); virObjectUnref(cfg); return ret; } @@ -1247,8 +1235,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1273,8 +1260,7 @@ libxlDomainGetState(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1410,8 +1396,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, virDomainObjListRemove(driver-domains, vm); vm = NULL; } -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1466,8 +1451,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, if (VIR_CLOSE(fd) 0) virReportSystemError(errno, %s, _(cannot close file)); virDomainDefFree(def); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); virObjectUnref(cfg); return ret; @@ -1573,8 +1557,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) virDomainObjListRemove(driver-domains, vm); vm = NULL; } -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) libxlDomainEventQueue(driver, event); return ret; @@ -1633,8 +1616,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) virDomainObjListRemove(driver-domains, vm); vm = NULL; } -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); VIR_FREE(name); return ret; } @@ -1678,8 +1660,7 @@ libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned
[libvirt] [PATCH 06/12] openvz: virObjectUnlock usage cleanup
---
src/openvz/openvz_driver.c | 69 --
1 file changed, 23 insertions(+), 46 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 87df2a7..2f410c3 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -288,8 +288,7 @@ openvzDomainGetHostname(virDomainPtr dom, unsigned int
flags)
}
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return hostname;
error:
@@ -319,8 +318,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr
conn,
dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -358,8 +356,7 @@ static char *openvzDomainGetOSType(virDomainPtr dom)
ignore_value(VIR_STRDUP(ret, vm-def-os.type));
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -385,8 +382,7 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr
conn,
dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -411,8 +407,7 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr
conn,
dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -454,8 +449,7 @@ static int openvzDomainGetInfo(virDomainPtr dom,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -485,8 +479,7 @@ openvzDomainGetState(virDomainPtr dom,
ret = openvzGetVEStatus(vm, state, reason);
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -507,8 +500,7 @@ static int openvzDomainIsActive(virDomainPtr dom)
ret = virDomainObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -529,8 +521,7 @@ static int openvzDomainIsPersistent(virDomainPtr dom)
ret = obj-persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -559,8 +550,7 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom,
unsigned int flags) {
ret = virDomainDefFormat(vm-def, flags);
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -618,8 +608,7 @@ static int openvzDomainSuspend(virDomainPtr dom)
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -657,8 +646,7 @@ static int openvzDomainResume(virDomainPtr dom)
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virObjectUnlock(vm);
return ret;
}
@@ -703,8 +691,7 @@ openvzDomainShutdownFlags(virDomainPtr dom,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -764,8 +751,7 @@ static int openvzDomainReboot(virDomainPtr dom,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1037,8 +1023,7 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml)
cleanup:
virDomainDefFree(vmdef);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
openvzDriverUnlock(driver);
return dom;
}
@@ -1125,8 +1110,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
cleanup:
virDomainDefFree(vmdef);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
openvzDriverUnlock(driver);
return dom;
}
@@ -1173,8 +1157,7 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned
int flags)
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1222,8 +1205,7 @@ openvzDomainUndefineFlags(virDomainPtr dom,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
openvzDriverUnlock(driver);
return ret;
}
@@ -1260,8 +1242,7 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart)
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1297,8 +1278,7 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart)
cleanup:
VIR_FREE(value);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1393,8 +1373,7 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom,
unsigned int nvcpus,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -2006,8 +1985,7 @@ openvzDomainInterfaceStats(virDomainPtr dom,
_(invalid path, '%s' is not a known interface), path);
cleanup:
-if (vm)
-virObjectUnlock(vm);
+
[libvirt] [PATCH 05/12] lxc: virObjectUnlock usage cleanup
--- src/lxc/lxc_driver.c | 129 +- src/lxc/lxc_process.c | 3 +- 2 files changed, 44 insertions(+), 88 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1086289..76ff824 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -263,8 +263,7 @@ static virDomainPtr lxcDomainLookupByID(virConnectPtr conn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -293,8 +292,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -320,8 +318,7 @@ static virDomainPtr lxcDomainLookupByName(virConnectPtr conn, dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -340,8 +337,7 @@ static int lxcDomainIsActive(virDomainPtr dom) ret = virDomainObjIsActive(obj); cleanup: -if (obj) -virObjectUnlock(obj); +virObjectUnlock(obj); return ret; } @@ -360,8 +356,7 @@ static int lxcDomainIsPersistent(virDomainPtr dom) ret = obj-persistent; cleanup: -if (obj) -virObjectUnlock(obj); +virObjectUnlock(obj); return ret; } @@ -379,8 +374,7 @@ static int lxcDomainIsUpdated(virDomainPtr dom) ret = obj-updated; cleanup: -if (obj) -virObjectUnlock(obj); +virObjectUnlock(obj); return ret; } @@ -502,8 +496,7 @@ static virDomainPtr lxcDomainDefineXML(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(def); virDomainDefFree(oldDef); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(caps); @@ -553,8 +546,7 @@ static int lxcDomainUndefineFlags(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(cfg); @@ -609,8 +601,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -635,8 +626,7 @@ lxcDomainGetState(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -655,8 +645,7 @@ static char *lxcDomainGetOSType(virDomainPtr dom) goto cleanup; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -676,8 +665,7 @@ lxcDomainGetMaxMemory(virDomainPtr dom) ret = vm-def-mem.max_balloon; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -702,8 +690,7 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -742,8 +729,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -872,8 +858,7 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); return ret; @@ -969,8 +954,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); virObjectUnref(caps); return ret; } @@ -994,8 +978,7 @@ static char *lxcDomainGetXMLDesc(virDomainPtr dom, flags); cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1085,8 +1068,7 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom, } cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(cfg); @@ -1199,8 +1181,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, cleanup: virDomainDefFree(def); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event) virObjectEventStateQueue(driver-domainEventState, event); virObjectUnref(caps); @@ -1274,8 +1255,7 @@ static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secla ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1449,8 +1429,7 @@ lxcDomainDestroyFlags(virDomainPtr dom, } cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); if (event)
[libvirt] [PATCH 00/12] Add explicit non-NULL check for virObjectUnlock()
Background is here: https://www.redhat.com/archives/libvir-list/2014-May/msg00613.html Roman Bogorodskiy (12): Add explicit non-NULL check for virObjectUnlock() qemu: virObjectUnlock usage cleanup bhyve: virObjectUnlock usage cleanup libxl: virObjectUnlock usage cleanup lxc: virObjectUnlock usage cleanup openvz: virObjectUnlock usage cleanup parallels: virObjectUnlock usage cleanup test: virObjectUnlock usage cleanup uml: virObjectUnlock usage cleanup vmware: virObjectUnlock usage cleanup util: virObjectUnlock usage cleanup maint: add virObjectUnlock to useless_free_options cfg.mk | 1 + src/bhyve/bhyve_driver.c | 9 +- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 124 +-- src/lxc/lxc_driver.c | 129 ++-- src/lxc/lxc_process.c| 3 +- src/openvz/openvz_driver.c | 69 +++-- src/parallels/parallels_driver.c | 30 ++-- src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_driver.c | 324 +-- src/qemu/qemu_migration.c| 12 +- src/qemu/qemu_process.c | 6 +- src/test/test_driver.c | 156 +++ src/uml/uml_driver.c | 81 -- src/util/virclosecallbacks.c | 3 +- src/util/virobject.c | 3 + src/vmware/vmware_driver.c | 51 ++ 17 files changed, 338 insertions(+), 669 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] qemu: virObjectUnlock usage cleanup
---
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_driver.c | 324 +++
src/qemu/qemu_migration.c| 12 +-
src/qemu/qemu_process.c | 6 +-
4 files changed, 115 insertions(+), 230 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 03d8842..e62fb35 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3229,8 +3229,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
-if (mon)
-virObjectUnlock(mon);
+virObjectUnlock(mon);
qemuMonitorClose(mon);
virCommandAbort(cmd);
virCommandFree(cmd);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cab653b..f8edb41 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -300,8 +300,7 @@ qemuAutostartDomain(virDomainObjPtr vm,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
virObjectUnref(cfg);
return ret;
}
@@ -1353,8 +1352,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -1382,8 +1380,7 @@ static virDomainPtr qemuDomainLookupByUUID(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -1409,8 +1406,7 @@ static virDomainPtr qemuDomainLookupByName(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -1429,8 +1425,7 @@ static int qemuDomainIsActive(virDomainPtr dom)
ret = virDomainObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1448,8 +1443,7 @@ static int qemuDomainIsPersistent(virDomainPtr dom)
ret = obj-persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1467,8 +1461,7 @@ static int qemuDomainIsUpdated(virDomainPtr dom)
ret = obj-updated;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1652,8 +1645,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr
conn,
cleanup:
virDomainDefFree(def);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event) {
qemuDomainEventQueue(driver, event);
if (event2)
@@ -1738,8 +1730,7 @@ static int qemuDomainSuspend(virDomainPtr dom)
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event)
qemuDomainEventQueue(driver, event);
@@ -1804,8 +1795,7 @@ static int qemuDomainResume(virDomainPtr dom)
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event)
qemuDomainEventQueue(driver, event);
virObjectUnref(caps);
@@ -1887,8 +1877,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1987,8 +1976,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -2030,8 +2018,7 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags)
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -2123,8 +2110,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event)
qemuDomainEventQueue(driver, event);
return ret;
@@ -2149,8 +2135,7 @@ static char *qemuDomainGetOSType(virDomainPtr dom) {
ignore_value(VIR_STRDUP(type, vm-def-os.type));
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return type;
}
@@ -2170,8 +2155,7 @@ qemuDomainGetMaxMemory(virDomainPtr dom)
ret = vm-def-mem.max_balloon;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -2277,8 +2261,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -2351,8 +2334,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr
dom, int period,
vm = NULL;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
@@ -2399,8 +2381,7 @@ static int qemuDomainInjectNMI(virDomainPtr
[libvirt] [PATCH 11/12] util: virObjectUnlock usage cleanup
--- src/util/virclosecallbacks.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c index 4f26172..b6eb5b5 100644 --- a/src/util/virclosecallbacks.c +++ b/src/util/virclosecallbacks.c @@ -343,8 +343,7 @@ virCloseCallbacksRun(virCloseCallbacksPtr closeCallbacks, } vm = list-entries[i].callback(vm, conn, opaque); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); } VIR_FREE(list-entries); VIR_FREE(list); -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] bhyve: virObjectUnlock usage cleanup
--- src/bhyve/bhyve_driver.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 580b124..af10ad3 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -522,8 +522,7 @@ bhyveDomainDefineXML(virConnectPtr conn, const char *xml) virObjectUnref(caps); virDomainDefFree(def); virDomainDefFree(oldDef); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -883,8 +882,7 @@ bhyveDomainCreateXML(virConnectPtr conn, cleanup: virObjectUnref(caps); virDomainDefFree(def); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -916,8 +914,7 @@ bhyveDomainDestroy(virDomainPtr dom) } cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/12] test: virObjectUnlock usage cleanup
---
src/test/test_driver.c | 156 +
1 file changed, 52 insertions(+), 104 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 37756e7..b7d1153 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1710,8 +1710,7 @@ static int testDomainIsActive(virDomainPtr dom)
ret = virDomainObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1731,8 +1730,7 @@ static int testDomainIsPersistent(virDomainPtr dom)
ret = obj-persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1781,8 +1779,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml,
ret-id = dom-def-id;
cleanup:
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
if (event)
testObjectEventQueue(privconn, event);
virDomainDefFree(def);
@@ -1812,8 +1809,7 @@ static virDomainPtr testDomainLookupByID(virConnectPtr
conn,
ret-id = dom-def-id;
cleanup:
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
return ret;
}
@@ -1838,8 +1834,7 @@ static virDomainPtr testDomainLookupByUUID(virConnectPtr
conn,
ret-id = dom-def-id;
cleanup:
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
return ret;
}
@@ -1864,8 +1859,7 @@ static virDomainPtr testDomainLookupByName(virConnectPtr
conn,
ret-id = dom-def-id;
cleanup:
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
return ret;
}
@@ -1912,8 +1906,7 @@ static int testDomainDestroy(virDomainPtr domain)
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event)
testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
@@ -1951,8 +1944,7 @@ static int testDomainResume(virDomainPtr domain)
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event) {
testDriverLock(privconn);
testObjectEventQueue(privconn, event);
@@ -1993,8 +1985,7 @@ static int testDomainSuspend(virDomainPtr domain)
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event) {
testDriverLock(privconn);
@@ -2042,8 +2033,7 @@ static int testDomainShutdownFlags(virDomainPtr domain,
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event)
testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
@@ -2118,8 +2108,7 @@ static int testDomainReboot(virDomainPtr domain,
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event)
testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
@@ -2158,8 +2147,7 @@ static int testDomainGetInfo(virDomainPtr domain,
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
return ret;
}
@@ -2189,8 +2177,7 @@ testDomainGetState(virDomainPtr domain,
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
return ret;
}
@@ -2290,8 +2277,7 @@ testDomainSaveFlags(virDomainPtr domain, const char *path,
VIR_FORCE_CLOSE(fd);
unlink(path);
}
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event)
testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
@@ -2396,8 +2382,7 @@ testDomainRestoreFlags(virConnectPtr conn,
virDomainDefFree(def);
VIR_FREE(xml);
VIR_FORCE_CLOSE(fd);
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
if (event)
testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
@@ -2474,8 +2459,7 @@ static int testDomainCoreDumpWithFormat(virDomainPtr
domain,
ret = 0;
cleanup:
VIR_FORCE_CLOSE(fd);
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
if (event)
testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
@@ -2523,8 +2507,7 @@ testDomainGetMaxMemory(virDomainPtr domain)
ret = privdom-def-mem.max_balloon;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
return ret;
}
@@ -2550,8 +2533,7 @@ static int testDomainSetMaxMemory(virDomainPtr domain,
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
return ret;
}
@@ -2581,8 +2563,7 @@ static int testDomainSetMemory(virDomainPtr domain,
ret = 0;
cleanup:
-if (privdom)
-virObjectUnlock(privdom);
+virObjectUnlock(privdom);
[libvirt] [PATCH 07/12] parallels: virObjectUnlock usage cleanup
--- src/parallels/parallels_driver.c | 30 ++ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ab59599..5e17bc5 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1117,8 +1117,7 @@ parallelsDomainLookupByID(virConnectPtr conn, int id) ret-id = dom-def-id; cleanup: -if (dom) -virObjectUnlock(dom); +virObjectUnlock(dom); return ret; } @@ -1146,8 +1145,7 @@ parallelsDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) ret-id = dom-def-id; cleanup: -if (dom) -virObjectUnlock(dom); +virObjectUnlock(dom); return ret; } @@ -1173,8 +1171,7 @@ parallelsDomainLookupByName(virConnectPtr conn, const char *name) ret-id = dom-def-id; cleanup: -if (dom) -virObjectUnlock(dom); +virObjectUnlock(dom); return ret; } @@ -1202,8 +1199,7 @@ parallelsDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) ret = 0; cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); return ret; } @@ -1225,8 +1221,7 @@ parallelsDomainGetOSType(virDomainPtr domain) ignore_value(VIR_STRDUP(ret, privdom-def-os.type)); cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); parallelsDriverUnlock(privconn); return ret; } @@ -1248,8 +1243,7 @@ parallelsDomainIsPersistent(virDomainPtr domain) ret = 1; cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); parallelsDriverUnlock(privconn); return ret; } @@ -1276,8 +1270,7 @@ parallelsDomainGetState(virDomainPtr domain, ret = 0; cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); return ret; } @@ -1306,8 +1299,7 @@ parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) ret = virDomainDefFormat(def, flags); cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); return ret; } @@ -1331,8 +1323,7 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) ret = 0; cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); return ret; } @@ -1374,8 +1365,7 @@ parallelsDomainChangeState(virDomainPtr domain, ret = 0; cleanup: -if (privdom) -virObjectUnlock(privdom); +virObjectUnlock(privdom); return ret; } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] uml: virObjectUnlock usage cleanup
---
src/uml/uml_driver.c | 81 ++--
1 file changed, 27 insertions(+), 54 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 1e0ec0e..3befd12 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -406,8 +406,7 @@ umlInotifyEvent(int watch,
}
}
}
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
if (event) {
umlDomainEventQueue(driver, event);
event = NULL;
@@ -748,8 +747,7 @@ static void umlProcessAutoDestroyDom(void *payload,
if (dom !dom-persistent)
virDomainObjListRemove(data-driver-domains, dom);
-if (dom)
-virObjectUnlock(dom);
+virObjectUnlock(dom);
if (event)
umlDomainEventQueue(data-driver, event);
virHashRemoveEntry(data-driver-autodestroy, uuidstr);
@@ -1372,8 +1370,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -1400,8 +1397,7 @@ static virDomainPtr umlDomainLookupByUUID(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -1428,8 +1424,7 @@ static virDomainPtr umlDomainLookupByName(virConnectPtr
conn,
if (dom) dom-id = vm-def-id;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return dom;
}
@@ -1454,8 +1449,7 @@ static int umlDomainIsActive(virDomainPtr dom)
ret = virDomainObjIsActive(obj);
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1480,8 +1474,7 @@ static int umlDomainIsPersistent(virDomainPtr dom)
ret = obj-persistent;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1505,8 +1498,7 @@ static int umlDomainIsUpdated(virDomainPtr dom)
ret = obj-updated;
cleanup:
-if (obj)
-virObjectUnlock(obj);
+virObjectUnlock(obj);
return ret;
}
@@ -1625,8 +1617,7 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr
conn, const char *xml,
cleanup:
virDomainDefFree(def);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event)
umlDomainEventQueue(driver, event);
umlDriverUnlock(driver);
@@ -1668,8 +1659,7 @@ static int umlDomainShutdownFlags(virDomainPtr dom,
cleanup:
VIR_FREE(info);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1714,8 +1704,7 @@ umlDomainDestroyFlags(virDomainPtr dom,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event)
umlDomainEventQueue(driver, event);
umlDriverUnlock(driver);
@@ -1750,8 +1739,7 @@ static char *umlDomainGetOSType(virDomainPtr dom) {
goto cleanup;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return type;
}
@@ -1782,8 +1770,7 @@ umlDomainGetMaxMemory(virDomainPtr dom)
ret = vm-def-mem.max_balloon;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1819,8 +1806,7 @@ static int umlDomainSetMaxMemory(virDomainPtr dom,
unsigned long newmax)
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1862,8 +1848,7 @@ static int umlDomainSetMemory(virDomainPtr dom, unsigned
long newmem)
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1905,8 +1890,7 @@ static int umlDomainGetInfo(virDomainPtr dom,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1940,8 +1924,7 @@ umlDomainGetState(virDomainPtr dom,
ret = 0;
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -1973,8 +1956,7 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom,
flags);
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
return ret;
}
@@ -2043,8 +2025,7 @@ static int umlDomainCreateWithFlags(virDomainPtr dom,
unsigned int flags)
VIR_DOMAIN_EVENT_STARTED_BOOTED);
cleanup:
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
if (event)
umlDomainEventQueue(driver, event);
umlDriverUnlock(driver);
@@ -2093,8 +2074,7 @@ static virDomainPtr umlDomainDefineXML(virConnectPtr
conn, const char *xml)
cleanup:
virDomainDefFree(def);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
umlDriverUnlock(driver);
return dom;
}
@@ -2138,8 +2118,7 @@ static int
[libvirt] [PATCH 12/12] maint: add virObjectUnlock to useless_free_options
Add virObjectUnlock() to a list of functions checking argument for NULL is useless for --- cfg.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/cfg.mk b/cfg.mk index 5ff2721..3ac67a8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -163,6 +163,7 @@ useless_free_options = \ --name=virNetworkObjFree \ --name=virNodeDeviceDefFree \ --name=virNodeDeviceObjFree \ + --name=virObjectUnlock\ --name=virObjectUnref \ --name=virObjectFreeCallback \ --name=virPCIDeviceFree \ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] vmware: virObjectUnlock usage cleanup
--- src/vmware/vmware_driver.c | 51 -- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 6edc0bc..4d8bf3c 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -401,8 +401,7 @@ vmwareDomainDefineXML(virConnectPtr conn, const char *xml) VIR_FREE(directoryName); VIR_FREE(fileName); VIR_FREE(vmxPath); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); vmwareDriverUnlock(driver); return dom; } @@ -446,8 +445,7 @@ vmwareDomainShutdownFlags(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); vmwareDriverUnlock(driver); return ret; } @@ -515,8 +513,7 @@ vmwareDomainSuspend(virDomainPtr dom) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -564,8 +561,7 @@ vmwareDomainResume(virDomainPtr dom) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -612,8 +608,7 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -685,8 +680,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, virDomainDefFree(vmdef); VIR_FREE(vmx); VIR_FREE(vmxPath); -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); vmwareDriverUnlock(driver); return dom; } @@ -723,8 +717,7 @@ vmwareDomainCreateWithFlags(virDomainPtr dom, ret = vmwareStartVM(driver, vm); cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); vmwareDriverUnlock(driver); return ret; } @@ -776,8 +769,7 @@ vmwareDomainUndefineFlags(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); vmwareDriverUnlock(driver); return ret; } @@ -809,8 +801,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -833,8 +824,7 @@ vmwareDomainGetOSType(virDomainPtr dom) ignore_value(VIR_STRDUP(ret, vm-def-os.type)); cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -860,8 +850,7 @@ vmwareDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -886,8 +875,7 @@ vmwareDomainLookupByName(virConnectPtr conn, const char *name) dom-id = vm-def-id; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return dom; } @@ -908,8 +896,7 @@ vmwareDomainIsActive(virDomainPtr dom) ret = virDomainObjIsActive(obj); cleanup: -if (obj) -virObjectUnlock(obj); +virObjectUnlock(obj); return ret; } @@ -931,8 +918,7 @@ vmwareDomainIsPersistent(virDomainPtr dom) ret = obj-persistent; cleanup: -if (obj) -virObjectUnlock(obj); +virObjectUnlock(obj); return ret; } @@ -959,8 +945,7 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) ret = virDomainDefFormat(vm-def, flags); cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1095,8 +1080,7 @@ vmwareDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } @@ -1129,8 +1113,7 @@ vmwareDomainGetState(virDomainPtr dom, ret = 0; cleanup: -if (vm) -virObjectUnlock(vm); +virObjectUnlock(vm); return ret; } -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] Misc error improvements
Collection of misc error improvements, see individual patches for details. Patch 2 and 3 have been ACK'd already. v2: Rebased to master Cole Robinson (3): virerror: Fix incorrect use of RaiseErrorFull virdbus: Remove redundant error macro virdbus: Show method name in error message src/util/virdbus.c | 12 +++--- src/util/virerror.h | 114 2 files changed, 41 insertions(+), 85 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] virdbus: Show method name in error message
If you trigger bug 1033369, we get the error message:
error from service: Invalid argument
Which is a bit too generic to pinpoint what is actually failing. This
changes it to:
error from service: CreateMachine: Invalid argument
Acked-by: Eric Blake ebl...@redhat.com
---
src/util/virdbus.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 03ec028..31251fe 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1408,7 +1408,8 @@ static int
virDBusCall(DBusConnection *conn,
DBusMessage *call,
DBusMessage **replyout,
-DBusError *error)
+DBusError *error,
+const char *member)
{
DBusMessage *reply = NULL;
DBusError localerror;
@@ -1424,7 +1425,7 @@ virDBusCall(DBusConnection *conn,
if (error)
ret = 0;
else {
-virReportError(VIR_ERR_DBUS_SERVICE, %s,
+virReportError(VIR_ERR_DBUS_SERVICE, _(%s: %s), member,
localerror.message ? localerror.message : _(unknown error));
}
goto cleanup;
@@ -1502,7 +1503,7 @@ int virDBusCallMethod(DBusConnection *conn,
ret = -1;
-ret = virDBusCall(conn, call, replyout, error);
+ret = virDBusCall(conn, call, replyout, error, member);
cleanup:
if (call)
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] virerror: Fix incorrect use of RaiseErrorFull
RaiseErrorFull does not prepend the static error code string (like INVALID_ARG yields invalid arg: %(msg)s). We should be using ReportErrorHelper. The generated error objects are slightly different, by not storing the invalid argument name in err-str2. However those fields aren't used anywhere else and aren't documented to contain anything useful, so I don't think it matters. Cc: Daniel P. Berrange berra...@redhat.com --- Dan, can you ACK? Eric and Guido requested your explicit review: http://www.redhat.com/archives/libvir-list/2014-May/msg00118.html http://www.redhat.com/archives/libvir-list/2014-May/msg00150.html src/util/virerror.h | 116 +--- 1 file changed, 38 insertions(+), 78 deletions(-) diff --git a/src/util/virerror.h b/src/util/virerror.h index fe0e15e..872c270 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -69,92 +69,52 @@ void virReportSystemErrorFull(int domcode, (fmt), __VA_ARGS__) # define virReportInvalidNullArg(argname)\ -virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _(%s in %s must be NULL),\ - #argname, __FUNCTION__) +virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG,\ + __FILE__, __FUNCTION__, __LINE__, \ + _(%s in %s must be NULL), \ + #argname, __FUNCTION__) # define virReportInvalidNonNullArg(argname) \ -virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _(%s in %s must not be NULL),\ - #argname, __FUNCTION__) +virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG,\ + __FILE__, __FUNCTION__, __LINE__, \ + _(%s in %s must not be NULL), \ + #argname, __FUNCTION__) # define virReportInvalidPositiveArg(argname)\ -virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _(%s in %s must be greater than zero), \ - #argname, __FUNCTION__) +virReportErrorHelper(VIR_FROM_THIS, \ + VIR_ERR_INVALID_ARG,\ + __FILE__, __FUNCTION__, __LINE__, \ + _(%s in %s must be greater than zero),\ + #argname, __FUNCTION__) # define virReportInvalidNonZeroArg(argname) \ -virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ - VIR_FROM_THIS, \ - VIR_ERR_INVALID_ARG, \ - VIR_ERR_ERROR, \ - __FUNCTION__, \ - #argname, \ - NULL, \ - 0, 0, \ - _(%s in %s
[libvirt] [PATCH v2 2/3] virdbus: Remove redundant error macro
This is the only callsite.
We drop use of localerror.name here, because it's not actually useful
to us: rather than the parameter name which received an invalid value
(which was assumed), it's actually the the dbus errno equivalent.
Just use the string.
Acked-by: Eric Blake ebl...@redhat.com
---
src/util/virdbus.c | 7 ---
src/util/virerror.h | 6 --
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 709d6ee..03ec028 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1423,9 +1423,10 @@ virDBusCall(DBusConnection *conn,
error ? error :
localerror))) {
if (error)
ret = 0;
-else
-virReportDBusServiceError(localerror.message ? localerror.message
: unknown error,
- localerror.name);
+else {
+virReportError(VIR_ERR_DBUS_SERVICE, %s,
+localerror.message ? localerror.message : _(unknown error));
+}
goto cleanup;
}
diff --git a/src/util/virerror.h b/src/util/virerror.h
index 872c270..4b01b77 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -110,12 +110,6 @@ void virReportSystemErrorFull(int domcode,
__FILE__, __FUNCTION__, __LINE__, \
(fmt), __VA_ARGS__)
-# define virReportDBusServiceError(message, name)\
-virReportErrorHelper(VIR_FROM_THIS, \
- VIR_ERR_DBUS_SERVICE, \
- __FILE__, __FUNCTION__, __LINE__, \
- %s, message)
-
# define virReportUnsupportedError()\
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT, \
__FILE__, __FUNCTION__, __LINE__, __FUNCTION__)
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bhyve: fix virObjectUnlock() usage
On Mon, May 19, 2014 at 11:12:05AM -0600, Eric Blake wrote: On 05/19/2014 10:57 AM, Roman Bogorodskiy wrote: Eric Blake wrote: On 05/17/2014 01:44 PM, Roman Bogorodskiy wrote: In a number of places in the bhyve driver, virObjectUnlock() is called with an arg without check if the arg is non-NULL, which could result in passing NULL value and a warning like: virObjectUnlock:340 : Object 0x0 ((unknown)) is not a virObjectLockable instance Doesn't this instead argue that we should fix virObjectUnlock to gracefully handle a NULL parameter, rather than making every caller uglier? Calling it with NULL seems to be harmless anyway and the only problem is this annoying warning. So, what you propose is checking explicitly for NULL in virObjectUnlock before we do virObjectIsClass(), and if the passed argument is NULL indeed, just return, without logging anything about that? Yes, since we have other virObject code that special cases NULL (for example, virObjectUnref). IMHO passing NULL into the locking APIs is a coding error we shouldn't try to paper over by ignoring. Ultimately I think the real flaw is the way we obtain the virDomainPtr pointers in the first place. ie the virDomainObjListLookup functions don't acquire a reference on the object they return. So the caller has to worry about the object being released behind their back, hence all our logic which has to set 'vm = NULL' in various places where it might have been deleted. IOW, I'd much rather we looked at changing our design here so that we didn't have so much NULL vm pointers in the first place. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/12] Add explicit non-NULL check for virObjectUnlock()
On Mon, May 19, 2014 at 10:27:29PM +0400, Roman Bogorodskiy wrote: Background is here: https://www.redhat.com/archives/libvir-list/2014-May/msg00613.html See my reply there - I don't think we we should be allowing NULL to be passed into the lock/unlock functions Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: nuke more uses of 'sync'
Commit d5c86278 was incomplete; other functions also triggered
compiler warnings about collisions in the use of 'sync'.
* src/qemu/qemu_driver.c (qemuDomainSetTime): Fix another client.
* tools/virsh-domain-monitor.c (cmdDomTime): Likewise.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Pushing under the build-breaker rule.
src/qemu/qemu_driver.c |4 ++--
tools/virsh-domain-monitor.c |6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cab653b..c8a0029 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16598,7 +16598,7 @@ qemuDomainSetTime(virDomainPtr dom,
virQEMUDriverPtr driver = dom-conn-privateData;
qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
-bool sync = flags VIR_DOMAIN_TIME_SYNC;
+bool rtcSync = flags VIR_DOMAIN_TIME_SYNC;
int ret = -1;
int rv;
@@ -16625,7 +16625,7 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
qemuDomainObjEnterAgent(vm);
-rv = qemuAgentSetTime(priv-agent, seconds, nseconds, sync);
+rv = qemuAgentSetTime(priv-agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
if (rv 0)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index a3b66ed..8bd58ad 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1400,7 +1400,7 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
bool ret = false;
bool now = vshCommandOptBool(cmd, now);
bool pretty = vshCommandOptBool(cmd, pretty);
-bool sync = vshCommandOptBool(cmd, sync);
+bool rtcSync = vshCommandOptBool(cmd, sync);
long long seconds = 0;
unsigned int nseconds = 0;
unsigned int flags = 0;
@@ -1426,13 +1426,13 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
doSet = true;
}
-if (doSet || now || sync) {
+if (doSet || now || rtcSync) {
if (now ((seconds = time(NULL)) == (time_t) -1)) {
vshError(ctl, _(Unable to get current time));
goto cleanup;
}
-if (sync)
+if (rtcSync)
flags |= VIR_DOMAIN_TIME_SYNC;
if (virDomainSetTime(dom, seconds, nseconds, flags) 0)
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix specifying char devs for PPC
Hi Cole, Thanks for the comments. Exactly there were already test cases for both pseries and ppce500 machines. For example, 1) qemuxml2argv-pseries-basic.args: '-chardev spapr-vty' for pseries. 2) qemuxml2argv-ppc-dtb.args: '-serial pty' for ppce500. Best Regards, Olivia -Original Message- From: Cole Robinson [mailto:crobi...@redhat.com] Sent: Monday, May 19, 2014 9:40 PM To: Yin Olivia-R63875; libvir-list@redhat.com; zhlci...@linux.vnet.ibm.com Subject: Re: [PATCH] qemu: Fix specifying char devs for PPC On 05/19/2014 03:41 AM, hong-hua@freescale.com wrote: Hi Cole, This is a patch similar with your previous patch to fix for ARM. Do you have any comments on it? Cindy, Since you are the main contributor to QEMU driver on PPC, I'll also appreciate your comments. Best Regards, Olivia Patch looks fine, but it should add a qemuxml2argv test case to validate that it actually works. My original patch added test cases later in the patch series, see 54a77c6df3c483864463f602c4c6f435d50bd79e - Cole -Original Message- From: Yin Olivia-R63875 Sent: Friday, May 16, 2014 8:38 AM To: Yin Olivia-R63875; libvir-list@redhat.com Subject: RE: [PATCH] qemu: Fix specifying char devs for PPC Ping. This is a patch similar with ARM platforms. http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=3a2beaee1d50dc96 8171c5 84ec2edcfdcb8fadde Right now on ppce500, chardev is not supported for the serial console. So it uses the the legacy -serial option. Best Regards, Olivia -Original Message- From: Olivia Yin [mailto:hong-hua@freescale.com] Sent: Wednesday, May 14, 2014 6:47 PM To: libvir-list@redhat.com Cc: Yin Olivia-R63875 Subject: [PATCH] qemu: Fix specifying char devs for PPC QEMU ppce500 board uses the old style -serial options. Other PPC boards don't give any way to explicitly wire in a -chardev except pseries which uses -device spapr-vty with -chardev. --- src/qemu/qemu_capabilities.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b491f58..fe5dd19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3460,13 +3460,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64) + (def-os.arch != VIR_ARCH_PPC) (def-os.arch != +VIR_ARCH_PPC64)) return true; /* This may not be true for all ARM machine types, but at least * the only supported non-virtio serial devices of vexpress and versatile - * don't have the -chardev property wired up. */ + * don't have the -chardev property wired up. + * For PPC machines, only pserial need -device spapr-vty with + -chardev */ return (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE - chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); + chr-targetType == + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) || +(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL + chr-info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); } -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
