Re: [libvirt] [PATCHv2] qemu: blockCopy: Allow reuse of raw image for shallow block copy
On 04/28/2015 09:53 AM, Peter Krempa wrote: > The documentation states that for shallow block copy the image has to > have the same guest visible content as backing file of the current > image if the file is being reused. This condition can be achieved also > with a raw file (or a qcow without a backing file) so remove the > condition that would disallow it. > > (This patch additionally fixes crash described in > https://bugzilla.redhat.com/show_bug.cgi?id=1215569 ) > --- > V2: > - different approach to fix Eric's comment > > > src/qemu/qemu_driver.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > ACK; and I think it qualifies as a bug fix so safe for freeze. -- 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] qemu: blockCopy: Allow shallow block copy into a raw image
On 04/27/2015 10:08 PM, Shanzhi Yu wrote: >>> Should libvirt post error when try a shallow blockcopy of file without >>> backing file, just as shallow blockcommit? >> I cannot reproduce the error above, could you please post steps to do >> that? or perhaps debug log from libvirt? > > It is easy to reproduce. Suppose live guest xml looks like: > > # virsh dumpxml vm1|grep disk -A 8 > > > > > > > > > > >function='0x0'/> > > > Then try a shallow blockcommit of file has no backing file > > But a shallow block copy of file which has backing file into a raw file > does not make any sense, right? A shallow block copy of: A <- B into a pre-existing file C, where C starts life with contents identical to A, makes sense. A shallow block copy where _libvirt_ creates C does not make sense. The key is that the moment you use the REUSE_EXTERNAL flag, _you_ are now responsible for providing a valid destination (and anything is supported; libvirt should not get in your way). -- 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] domain: conf: Drop unused OSTYPE_AIX
On 04/28/2015 06:46 PM, Cole Robinson wrote: > Ping... I think this should be safe for the release, since the AIX value has > never been exposed to the user ACK, and indeed worth having in the release > > - Cole > > On 04/22/2015 10:47 AM, Cole Robinson wrote: >> The phyp driver stuffed it into a DomainDefPtr during its attachdevice >> routine, but the value is never advertised via capabilities so it should >> be safe to drop. >> >> Have the phyp driver use OSTYPE_LINUX, which is what it advertises via >> capabilities. >> --- >> docs/schemas/capability.rng | 3 ++- >> src/conf/domain_conf.c | 3 +-- >> src/conf/domain_conf.h | 1 - >> src/phyp/phyp_driver.c | 2 +- >> tests/vircapstest.c | 2 +- >> 5 files changed, 5 insertions(+), 6 deletions(-) >> >> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng >> index 5f3ec70..88e08d2 100644 >> --- a/docs/schemas/capability.rng >> +++ b/docs/schemas/capability.rng >> @@ -262,7 +262,8 @@ >> >> >> xen >> -linux >> +linux >> hvm >> exe >> uml >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 479b4c2..6565350 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -120,8 +120,7 @@ VIR_ENUM_IMPL(virDomainOS, VIR_DOMAIN_OSTYPE_LAST, >>"xen", >>"linux", >>"exe", >> - "uml", >> - "aix") >> + "uml") >> >> VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST, >>"fd", >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h >> index 25d3ee6..7a374d7 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -233,7 +233,6 @@ typedef enum { >> VIR_DOMAIN_OSTYPE_LINUX, >> VIR_DOMAIN_OSTYPE_EXE, >> VIR_DOMAIN_OSTYPE_UML, >> -VIR_DOMAIN_OSTYPE_AIX, >> >> VIR_DOMAIN_OSTYPE_LAST >> } virDomainOSType; >> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c >> index e9a31d0..c558c48 100644 >> --- a/src/phyp/phyp_driver.c >> +++ b/src/phyp/phyp_driver.c >> @@ -1720,7 +1720,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char >> *xml) >> if (domain_name == NULL) >> goto cleanup; >> >> -def->os.type = VIR_DOMAIN_OSTYPE_AIX; >> +def->os.type = VIR_DOMAIN_OSTYPE_LINUX; >> >> dev = virDomainDeviceDefParse(xml, def, phyp_driver->caps, NULL, >>VIR_DOMAIN_DEF_PARSE_INACTIVE); >> diff --git a/tests/vircapstest.c b/tests/vircapstest.c >> index 5a43d63..0c79af8 100644 >> --- a/tests/vircapstest.c >> +++ b/tests/vircapstest.c >> @@ -250,7 +250,7 @@ test_virCapsDomainDataLookupQEMU(const void *data >> ATTRIBUTE_UNUSED) >> VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_PPC64LE, >> VIR_DOMAIN_VIRT_QEMU, "/usr/bin/qemu-system-ppc64", "pseries"); >> >> -CAPS_EXPECT_ERR(VIR_DOMAIN_OSTYPE_AIX, VIR_ARCH_NONE, -1, NULL, NULL); >> +CAPS_EXPECT_ERR(VIR_DOMAIN_OSTYPE_LINUX, VIR_ARCH_NONE, -1, NULL, NULL); >> CAPS_EXPECT_ERR(-1, VIR_ARCH_PPC64LE, -1, NULL, "pc"); >> CAPS_EXPECT_ERR(-1, VIR_ARCH_MIPS, -1, NULL, NULL); >> CAPS_EXPECT_ERR(-1, VIR_ARCH_AARCH64, VIR_DOMAIN_VIRT_KVM, >> > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > > -- 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] polkit: Allow password-less access for 'libvirt' group
On 04/28/2015 05:51 PM, Cole Robinson wrote: > Many users, who admin their own machines, want to be able to access > system libvirtd via tools like virt-manager without having to enter > a root password. Just google 'virt-manager without password' and > you'll find many hits. I've read at least 5 blog posts over the years > describing slightly different ways of achieving this goal. > > Let's finally add official support for this. > > Install a polkit-1 rules file granting password-less auth for any user > in the new 'libvirt' group. Create the group on RPM install > > https://bugzilla.redhat.com/show_bug.cgi?id=957300 > --- > daemon/50-libvirt.rules | 9 + Let's just name it daemon/libvirt.rules in git, and only rename it as part of installation (to make it a bit more obvious that someone could rename it to something other than 50- if they want a different priority). > daemon/Makefile.am | 13 + > libvirt.spec.in | 15 +-- > 3 files changed, 35 insertions(+), 2 deletions(-) > create mode 100644 daemon/50-libvirt.rules > -- 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 6/6] storage: fs: Only force directory permissions if required
On 04/28/2015 09:36 AM, Peter Krempa wrote: > On Mon, Apr 27, 2015 at 16:48:44 -0400, Cole Robinson wrote: >> Only set directory permissions at pool build time, if: >> >> - User explicitly requested a mode via the XML >> - The directory needs to be created >> - We need to do the crazy NFS root-squash workaround >> >> This allows qemu:///session to call build on an existing directory >> like /tmp. >> --- >> src/storage/storage_backend_fs.c | 16 +++- >> src/util/virfile.c | 2 +- >> 2 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/src/storage/storage_backend_fs.c >> b/src/storage/storage_backend_fs.c >> index 344ee4c..e11c240 100644 >> --- a/src/storage/storage_backend_fs.c >> +++ b/src/storage/storage_backend_fs.c >> @@ -769,6 +769,8 @@ virStorageBackendFileSystemBuild(virConnectPtr conn >> ATTRIBUTE_UNUSED, >> int err, ret = -1; >> char *parent = NULL; >> char *p = NULL; >> +mode_t mode; >> +bool needs_create_as_uid; >> >> virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | >>VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); >> @@ -802,19 +804,23 @@ virStorageBackendFileSystemBuild(virConnectPtr conn >> ATTRIBUTE_UNUSED, >> } >> } >> >> +needs_create_as_uid = (pool->def->type == VIR_STORAGE_POOL_NETFS); >> +mode = pool->def->target.perms.mode; >> +if (mode == (mode_t) -1 && >> +(needs_create_as_uid || !virFileExists(pool->def->target.path))) >> +mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; >> + >> /* Now create the final dir in the path with the uid/gid/mode >> * requested in the config. If the dir already exists, just set >> * the perms. */ >> if ((err = virDirCreate(pool->def->target.path, >> -(pool->def->target.perms.mode == (mode_t) -1 ? >> - VIR_STORAGE_DEFAULT_POOL_PERM_MODE : >> - pool->def->target.perms.mode), >> +mode, >> pool->def->target.perms.uid, >> pool->def->target.perms.gid, >> VIR_DIR_CREATE_FORCE_PERMS | >> VIR_DIR_CREATE_ALLOW_EXIST | >> -(pool->def->type == VIR_STORAGE_POOL_NETFS >> -? VIR_DIR_CREATE_AS_UID : 0))) < 0) { >> +(needs_create_as_uid ? VIR_DIR_CREATE_AS_UID : >> 0) >> +)) < 0) { > > Closing parentheses on a separate line are ugly. I'd rather see > violating the 80 cols rule rather than damaging readability of the code. > >> goto error; >> } >> >> diff --git a/src/util/virfile.c b/src/util/virfile.c >> index 676e7b4..7e49f39 100644 >> --- a/src/util/virfile.c >> +++ b/src/util/virfile.c >> @@ -2311,7 +2311,7 @@ virDirCreateNoFork(const char *path, >> path, (unsigned int) uid, (unsigned int) gid); >> goto error; >> } >> -if ((flags & VIR_DIR_CREATE_FORCE_PERMS) >> +if (((flags & VIR_DIR_CREATE_FORCE_PERMS) && mode != (mode_t) -1) > > This is a usage error of the function. Using mode == -1 and requesting > it to be set doesn't make much sense. > And to clarify, I'll push patches 1-4 when the new release opens, since they had minor changes. Then I'll post a new series with 5-6 + additional patches - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] storage: fs: Only force directory permissions if required
On 04/28/2015 09:36 AM, Peter Krempa wrote: > On Mon, Apr 27, 2015 at 16:48:44 -0400, Cole Robinson wrote: >> Only set directory permissions at pool build time, if: >> >> - User explicitly requested a mode via the XML >> - The directory needs to be created >> - We need to do the crazy NFS root-squash workaround >> >> This allows qemu:///session to call build on an existing directory >> like /tmp. >> --- >> src/storage/storage_backend_fs.c | 16 +++- >> src/util/virfile.c | 2 +- >> 2 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/src/storage/storage_backend_fs.c >> b/src/storage/storage_backend_fs.c >> index 344ee4c..e11c240 100644 >> --- a/src/storage/storage_backend_fs.c >> +++ b/src/storage/storage_backend_fs.c >> @@ -769,6 +769,8 @@ virStorageBackendFileSystemBuild(virConnectPtr conn >> ATTRIBUTE_UNUSED, >> int err, ret = -1; >> char *parent = NULL; >> char *p = NULL; >> +mode_t mode; >> +bool needs_create_as_uid; >> >> virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | >>VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); >> @@ -802,19 +804,23 @@ virStorageBackendFileSystemBuild(virConnectPtr conn >> ATTRIBUTE_UNUSED, >> } >> } >> >> +needs_create_as_uid = (pool->def->type == VIR_STORAGE_POOL_NETFS); >> +mode = pool->def->target.perms.mode; >> +if (mode == (mode_t) -1 && >> +(needs_create_as_uid || !virFileExists(pool->def->target.path))) >> +mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; >> + >> /* Now create the final dir in the path with the uid/gid/mode >> * requested in the config. If the dir already exists, just set >> * the perms. */ >> if ((err = virDirCreate(pool->def->target.path, >> -(pool->def->target.perms.mode == (mode_t) -1 ? >> - VIR_STORAGE_DEFAULT_POOL_PERM_MODE : >> - pool->def->target.perms.mode), >> +mode, >> pool->def->target.perms.uid, >> pool->def->target.perms.gid, >> VIR_DIR_CREATE_FORCE_PERMS | >> VIR_DIR_CREATE_ALLOW_EXIST | >> -(pool->def->type == VIR_STORAGE_POOL_NETFS >> -? VIR_DIR_CREATE_AS_UID : 0))) < 0) { >> +(needs_create_as_uid ? VIR_DIR_CREATE_AS_UID : >> 0) >> +)) < 0) { > > Closing parentheses on a separate line are ugly. I'd rather see > violating the 80 cols rule rather than damaging readability of the code. > Will do >> goto error; >> } >> >> diff --git a/src/util/virfile.c b/src/util/virfile.c >> index 676e7b4..7e49f39 100644 >> --- a/src/util/virfile.c >> +++ b/src/util/virfile.c >> @@ -2311,7 +2311,7 @@ virDirCreateNoFork(const char *path, >> path, (unsigned int) uid, (unsigned int) gid); >> goto error; >> } >> -if ((flags & VIR_DIR_CREATE_FORCE_PERMS) >> +if (((flags & VIR_DIR_CREATE_FORCE_PERMS) && mode != (mode_t) -1) > > This is a usage error of the function. Using mode == -1 and requesting > it to be set doesn't make much sense. > Hmm, I think instead I'll add an additional patch to drop FORCE_PERMS entirely.. it's used by every virDirCreate caller. We can just key the chmod off of whether mode != -1 >> && (chmod(path, mode) < 0)) { >> ret = -errno; >> virReportSystemError(errno, > > ACK. > > Peter > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] storage: conf: Don't set any default in the XML
On 04/28/2015 09:23 AM, Peter Krempa wrote: > On Mon, Apr 27, 2015 at 16:48:43 -0400, Cole Robinson wrote: >> The XML parser sets a default if none is explicitly passed in. >> This is then used at pool/vol creation time, and unconditionally reported >> in the XML. >> >> The problem with this approach is that it's impossible for other code >> to determine if the user explicitly requested a storage mode. There >> are some cases where we want to make this distinction, but we currently >> can't. >> >> Handle parsing like we handle /: if no value is >> passed in, set it to -1, and adjust the internal consumers to handle >> it. >> --- >> docs/schemas/storagecommon.rng | 5 ++- >> src/conf/storage_conf.c| 42 >> +++--- >> src/storage/storage_backend.c | 20 --- >> src/storage/storage_backend.h | 3 ++ >> src/storage/storage_backend_fs.c | 9 +++-- >> src/storage/storage_backend_logical.c | 4 ++- >> tests/storagepoolxml2xmlin/pool-dir.xml| 2 +- >> tests/storagepoolxml2xmlout/pool-dir.xml | 2 +- >> tests/storagepoolxml2xmlout/pool-netfs-gluster.xml | 2 +- >> tests/storagevolxml2xmlin/vol-file.xml | 6 ++-- >> tests/storagevolxml2xmlout/vol-file.xml| 6 ++-- >> tests/storagevolxml2xmlout/vol-gluster-dir.xml | 2 +- >> tests/storagevolxml2xmlout/vol-sheepdog.xml| 2 +- >> 13 files changed, 64 insertions(+), 41 deletions(-) >> >> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng >> index 5f71b10..e4e8a51 100644 >> --- a/docs/schemas/storagecommon.rng >> +++ b/docs/schemas/storagecommon.rng >> @@ -99,7 +99,10 @@ >> >> >> >> - >> + >> + >> + -1 >> + > > I'd rather make the mode optional if you want to keep the default value. > >> >> >> >> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c >> index 4852dfb..7131242 100644 >> --- a/src/conf/storage_conf.c >> +++ b/src/conf/storage_conf.c >> @@ -50,9 +50,6 @@ >> >> VIR_LOG_INIT("conf.storage_conf"); >> >> -#define DEFAULT_POOL_PERM_MODE 0755 >> -#define DEFAULT_VOL_PERM_MODE 0600 >> - >> VIR_ENUM_IMPL(virStorageVol, >>VIR_STORAGE_VOL_LAST, >>"file", "block", "dir", "network", "netdir") >> @@ -718,8 +715,7 @@ virStoragePoolDefParseSourceString(const char *srcSpec, >> static int >> virStorageDefParsePerms(xmlXPathContextPtr ctxt, >> virStoragePermsPtr perms, >> -const char *permxpath, >> -int defaultmode) >> +const char *permxpath) >> { >> char *mode; >> long long val; >> @@ -730,7 +726,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, >> node = virXPathNode(permxpath, ctxt); >> if (node == NULL) { >> /* Set default values if there is not element */ >> -perms->mode = defaultmode; >> +perms->mode = (mode_t) -1; >> perms->uid = (uid_t) -1; >> perms->gid = (gid_t) -1; >> perms->label = NULL; >> @@ -740,13 +736,12 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, >> relnode = ctxt->node; >> ctxt->node = node; >> >> -mode = virXPathString("string(./mode)", ctxt); >> -if (!mode) { >> -perms->mode = defaultmode; >> -} else { >> +if ((mode = virXPathString("string(./mode)", ctxt))) { >> int tmp; >> >> -if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || (tmp & ~0777)) { >> +if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || >> +((tmp & ~0777) && >> + tmp != -1)) { >> VIR_FREE(mode); >> virReportError(VIR_ERR_XML_ERROR, "%s", >> _("malformed octal mode")); >> @@ -754,6 +749,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, >> } >> perms->mode = tmp; >> VIR_FREE(mode); >> +} else { >> +perms->mode = (mode_t) -1; >> } >> >> if (virXPathNode("./owner", ctxt) == NULL) { >> @@ -947,8 +944,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) >> goto error; >> >> if (virStorageDefParsePerms(ctxt, &ret->target.perms, >> -"./target/permissions", >> -DEFAULT_POOL_PERM_MODE) < 0) >> +"./target/permissions") < 0) >> goto error; >> } >> >> @@ -1185,8 +1181,11 @@ virStoragePoolDefFormatBuf(virBufferPtr buf, >> >> virBufferAddLit(buf, "\n"); >> virBufferAdjustIndent(buf, 2); >> -virBufferAsprintf(buf, "0%o\n", >> - def->target.perms.mode); >> +if (def->target.perms.mode == (mode_t) -1) >> +v
Re: [libvirt] [PATCH 3/6] storage: fs: Don't attempt directory creation if it already exists
On 04/28/2015 08:19 AM, Peter Krempa wrote: > On Mon, Apr 27, 2015 at 16:48:41 -0400, Cole Robinson wrote: >> The current code attempts to handle this, but it only catches mkdir >> failing with EEXIST. However if say trying to build /tmp for an >> unprivileged qemu:///session, mkdir will fail with EPERM. >> >> Rather than catch any errors, just don't attempt mkdir if the directory >> already exists. >> --- >> src/util/virfile.c | 13 +++-- >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/src/util/virfile.c b/src/util/virfile.c >> index 87d121d..23a1655 100644 >> --- a/src/util/virfile.c >> +++ b/src/util/virfile.c >> @@ -2289,12 +2289,13 @@ virDirCreateNoFork(const char *path, >> int ret = 0; >> struct stat st; >> >> -if ((mkdir(path, mode) < 0) >> -&& !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { >> -ret = -errno; >> -virReportSystemError(errno, _("failed to create directory '%s'"), >> - path); >> -goto error; >> +if (!(flags & VIR_DIR_CREATE_ALLOW_EXIST) || !virFileExists(path)) { > > De Morgan's law allows to optimize this into a more readable form: > if (!(virFileExists(path) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { > > It's one set of parentheses more but it's readable. > Huh, personally I find that form much less readable. You need to mentally evaluate the parenthesized expression, then remember to take the opposite. The first one I can read out loud from left to right: if not ALLOW_EXIST or not file exists: stuff Maybe I've just written too much python :) Regardless I've applied your suggestion locally, will push after the release. Thanks, Cole >> +if (mkdir(path, mode) < 0) { >> +ret = -errno; >> +virReportSystemError(errno, _("failed to create directory >> '%s'"), >> + path); >> +goto error; >> +} > > ACK with the condition made human readable. > > Peter > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] storage: fs: Fill in permissions on pool refresh
On 04/28/2015 08:06 AM, Peter Krempa wrote: > On Mon, Apr 27, 2015 at 16:48:40 -0400, Cole Robinson wrote: >> This means pool XML actually reports accurate user/group/mode/label. >> >> This uses UpdateVolTargetInfoFD in a bit of a hackish way, but it works >> --- >> src/storage/storage_backend_fs.c | 58 >> ++-- >> 1 file changed, 44 insertions(+), 14 deletions(-) >> >> diff --git a/src/storage/storage_backend_fs.c >> b/src/storage/storage_backend_fs.c >> index 51d6bb3..804b7c3 100644 >> --- a/src/storage/storage_backend_fs.c > > ... > >> +pool->def->target.perms.mode = target->perms->mode; >> +pool->def->target.perms.uid = target->perms->uid; >> +pool->def->target.perms.gid = target->perms->gid; >> +VIR_FREE(pool->def->target.perms.label); >> +if (VIR_STRDUP(pool->def->target.perms.label, target->perms->label) < 0) >> +goto error; >> >> +ret = 0; >> error: > > Since both success and error paths use this label now, it should be > called 'cleanup'; > >> if (dir) >> closedir(dir); >> +VIR_FORCE_CLOSE(fd); >> virStorageVolDefFree(vol); >> -virStoragePoolObjClearVols(pool); >> -return -1; >> +virStorageSourceFree(target); >> +if (ret < 0) >> +virStoragePoolObjClearVols(pool); >> +return ret; >> } > > ACK with the label renamed. > > Peter > Thanks, applied that change locally. Will push after the release is out - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain: conf: Drop unused OSTYPE_AIX
Ping... I think this should be safe for the release, since the AIX value has never been exposed to the user - Cole On 04/22/2015 10:47 AM, Cole Robinson wrote: > The phyp driver stuffed it into a DomainDefPtr during its attachdevice > routine, but the value is never advertised via capabilities so it should > be safe to drop. > > Have the phyp driver use OSTYPE_LINUX, which is what it advertises via > capabilities. > --- > docs/schemas/capability.rng | 3 ++- > src/conf/domain_conf.c | 3 +-- > src/conf/domain_conf.h | 1 - > src/phyp/phyp_driver.c | 2 +- > tests/vircapstest.c | 2 +- > 5 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng > index 5f3ec70..88e08d2 100644 > --- a/docs/schemas/capability.rng > +++ b/docs/schemas/capability.rng > @@ -262,7 +262,8 @@ > > > xen > -linux > +linux > hvm > exe > uml > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 479b4c2..6565350 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -120,8 +120,7 @@ VIR_ENUM_IMPL(virDomainOS, VIR_DOMAIN_OSTYPE_LAST, >"xen", >"linux", >"exe", > - "uml", > - "aix") > + "uml") > > VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST, >"fd", > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 25d3ee6..7a374d7 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -233,7 +233,6 @@ typedef enum { > VIR_DOMAIN_OSTYPE_LINUX, > VIR_DOMAIN_OSTYPE_EXE, > VIR_DOMAIN_OSTYPE_UML, > -VIR_DOMAIN_OSTYPE_AIX, > > VIR_DOMAIN_OSTYPE_LAST > } virDomainOSType; > diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c > index e9a31d0..c558c48 100644 > --- a/src/phyp/phyp_driver.c > +++ b/src/phyp/phyp_driver.c > @@ -1720,7 +1720,7 @@ phypDomainAttachDevice(virDomainPtr domain, const char > *xml) > if (domain_name == NULL) > goto cleanup; > > -def->os.type = VIR_DOMAIN_OSTYPE_AIX; > +def->os.type = VIR_DOMAIN_OSTYPE_LINUX; > > dev = virDomainDeviceDefParse(xml, def, phyp_driver->caps, NULL, >VIR_DOMAIN_DEF_PARSE_INACTIVE); > diff --git a/tests/vircapstest.c b/tests/vircapstest.c > index 5a43d63..0c79af8 100644 > --- a/tests/vircapstest.c > +++ b/tests/vircapstest.c > @@ -250,7 +250,7 @@ test_virCapsDomainDataLookupQEMU(const void *data > ATTRIBUTE_UNUSED) > VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_PPC64LE, > VIR_DOMAIN_VIRT_QEMU, "/usr/bin/qemu-system-ppc64", "pseries"); > > -CAPS_EXPECT_ERR(VIR_DOMAIN_OSTYPE_AIX, VIR_ARCH_NONE, -1, NULL, NULL); > +CAPS_EXPECT_ERR(VIR_DOMAIN_OSTYPE_LINUX, VIR_ARCH_NONE, -1, NULL, NULL); > CAPS_EXPECT_ERR(-1, VIR_ARCH_PPC64LE, -1, NULL, "pc"); > CAPS_EXPECT_ERR(-1, VIR_ARCH_MIPS, -1, NULL, NULL); > CAPS_EXPECT_ERR(-1, VIR_ARCH_AARCH64, VIR_DOMAIN_VIRT_KVM, > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Make sure permissions are set on VNC auto socket
This can cause permissions failures if qemu.conf user/group is changed. Fix this by filling in the VNC auto socket path in the same code area that we fill in default listen address, ports, etc. https://bugzilla.redhat.com/show_bug.cgi?id=1151762 --- src/qemu/qemu_command.c | 10 ++ src/qemu/qemu_process.c | 11 ++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba15dc9..45fc63c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7544,7 +7544,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, static int qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, -virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainGraphicsDefPtr graphics) { @@ -7561,12 +7560,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } -if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { -if (!graphics->data.vnc.socket && -virAsprintf(&graphics->data.vnc.socket, -"%s/%s.vnc", cfg->libDir, def->name) == -1) -goto error; - +if (graphics->data.vnc.socket) { virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { @@ -7944,7 +7938,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_VNC: -return qemuBuildGraphicsVNCCommandLine(cfg, cmd, def, qemuCaps, graphics); +return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics); case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 605b3c6..5de46e2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4479,7 +4479,16 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } -if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || +if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && +!graphics->data.vnc.socket && +cfg->vncAutoUnixSocket) { +if (virAsprintf(&graphics->data.vnc.socket, +"%s/%s.vnc", cfg->libDir, vm->def->name) < 0) +goto cleanup; +} + +if ((graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket) || graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (graphics->nListens == 0) { if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] polkit: Allow password-less access for 'libvirt' group
Many users, who admin their own machines, want to be able to access system libvirtd via tools like virt-manager without having to enter a root password. Just google 'virt-manager without password' and you'll find many hits. I've read at least 5 blog posts over the years describing slightly different ways of achieving this goal. Let's finally add official support for this. Install a polkit-1 rules file granting password-less auth for any user in the new 'libvirt' group. Create the group on RPM install https://bugzilla.redhat.com/show_bug.cgi?id=957300 --- daemon/50-libvirt.rules | 9 + daemon/Makefile.am | 13 + libvirt.spec.in | 15 +-- 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 daemon/50-libvirt.rules diff --git a/daemon/50-libvirt.rules b/daemon/50-libvirt.rules new file mode 100644 index 000..01a15fa --- /dev/null +++ b/daemon/50-libvirt.rules @@ -0,0 +1,9 @@ +// Allow any user in the 'libvirt' group to connect to system libvirtd +// without entering a password. + +polkit.addRule(function(action, subject) { +if (action.id == "org.libvirt.unix.manage" && +subject.isInGroup("libvirt")) { +return polkit.Result.YES; +} +}); diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 300b9a5..e200ac1 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -53,6 +53,7 @@ EXTRA_DIST = \ libvirtd.init.in\ libvirtd.upstart\ libvirtd.policy.in \ + 50-libvirt.rules\ libvirtd.sasl \ libvirtd.service.in \ libvirtd.socket.in \ @@ -233,6 +234,8 @@ policyauth = auth_admin_keep_session else ! WITH_POLKIT0 policydir = $(datadir)/polkit-1/actions policyauth = auth_admin_keep +rulesdir = $(datadir)/polkit-1/rules.d +rulesfile = 50-libvirt.rules endif ! WITH_POLKIT0 endif WITH_POLKIT @@ -263,9 +266,19 @@ if WITH_POLKIT install-data-polkit:: $(MKDIR_P) $(DESTDIR)$(policydir) $(INSTALL_DATA) libvirtd.policy $(DESTDIR)$(policydir)/org.libvirt.unix.policy +if ! WITH_POLKIT0 + $(MKDIR_P) $(DESTDIR)$(rulesdir) + $(INSTALL_DATA) $(srcdir)/$(rulesfile) $(DESTDIR)$(rulesdir) +endif ! WITH_POLKIT0 + uninstall-data-polkit:: rm -f $(DESTDIR)$(policydir)/org.libvirt.unix.policy rmdir $(DESTDIR)$(policydir) || : +if ! WITH_POLKIT0 + rm -f $(DESTDIR)$(rulesdir)/$(rulesfile) + rmdir $(DESTDIR)$(rulesdir) +endif ! WITH_POLKIT0 + else ! WITH_POLKIT install-data-polkit:: uninstall-data-polkit:: diff --git a/libvirt.spec.in b/libvirt.spec.in index 20af502..c71ef25 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1645,9 +1645,9 @@ then fi %if %{with_libvirtd} +%pre daemon %if ! %{with_driver_modules} %if %{with_qemu} -%pre daemon %if 0%{?fedora} || 0%{?rhel} >= 6 # We want soft static allocation of well-known ids, as disk images # are commonly shared across NFS mounts by id rather than name; see @@ -1661,11 +1661,21 @@ if ! getent passwd qemu >/dev/null; then useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu fi fi -exit 0 %endif %endif %endif +%if %{with_polkit} +%if 0%{?fedora} || 0%{?rhel} >= 6 +# 'libvirt' group is just to allow password-less polkit access to +# libvirtd. The uid number is irrelevant, so we use dynamic allocation +# described at the above link. +getent group libvirt >/dev/null || groupadd -r libvirt +%endif +%endif + +exit 0 + %post daemon %if %{with_systemd} @@ -1939,6 +1949,7 @@ exit 0 %if 0%{?fedora} || 0%{?rhel} >= 6 %{_datadir}/polkit-1/actions/org.libvirt.unix.policy %{_datadir}/polkit-1/actions/org.libvirt.api.policy +%{_datadir}/polkit-1/rules.d/50-libvirt.rules %else %{_datadir}/PolicyKit/policy/org.libvirt.unix.policy %endif -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] libxl: provide integration with lock manager
Jim Fehlig wrote: > Provide integration with libvirt's lock manager in the libxl driver. > > Signed-off-by: Jim Fehlig > --- > > V3 of > > https://www.redhat.com/archives/libvir-list/2015-April/msg01006.html > > In V3, call virDomainLockProcessStart() with 'paused' parameter > set to 'true' and add a call to virDomainLockProcessResume(). > > src/Makefile.am | 12 + > src/libxl/libvirtd_libxl.aug | 2 ++ > src/libxl/libxl.conf | 10 +++ > src/libxl/libxl_conf.c | 14 ++ > src/libxl/libxl_conf.h | 6 + > src/libxl/libxl_domain.c | 51 > +++- > src/libxl/libxl_domain.h | 1 + > src/libxl/libxl_driver.c | 25 ++ > src/libxl/libxl_migration.c | 6 + > src/libxl/test_libvirtd_libxl.aug.in | 1 + > 10 files changed, 127 insertions(+), 1 deletion(-) > Any comments on this patch? The other two patches in original V1 have been pushed, and I'd like to push this one for 1.2.15 if there are no additional comments. Regards, Jim > diff --git a/src/Makefile.am b/src/Makefile.am > index 9a5f16c..1438174 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf > DISTCLEANFILES += locking/qemu-lockd.conf > endif WITH_QEMU > > +if WITH_LIBXL > +nodist_conf_DATA += locking/libxl-lockd.conf > +BUILT_SOURCES += locking/libxl-lockd.conf > +DISTCLEANFILES += locking/libxl-lockd.conf > +endif WITH_LIBXL > + > locking/%-lockd.conf: $(srcdir)/locking/lockd.conf > $(AM_V_GEN)$(MKDIR_P) locking ; \ > cp $< $@ > @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf > BUILT_SOURCES += locking/qemu-sanlock.conf > DISTCLEANFILES += locking/qemu-sanlock.conf > endif WITH_QEMU > + > +if WITH_LIBXL > +nodist_conf_DATA += locking/libxl-sanlock.conf > +BUILT_SOURCES += locking/libxl-sanlock.conf > +DISTCLEANFILES += locking/libxl-sanlock.conf > +endif WITH_LIBXL > else ! WITH_SANLOCK > EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) > endif ! WITH_SANLOCK > diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug > index f225954..d5aa150 100644 > --- a/src/libxl/libvirtd_libxl.aug > +++ b/src/libxl/libvirtd_libxl.aug > @@ -25,9 +25,11 @@ module Libvirtd_libxl = > > (* Config entry grouped by function - same order as example config *) > let autoballoon_entry = bool_entry "autoballoon" > + let lock_entry = str_entry "lock_manager" > > (* Each entry in the config is one of the following ... *) > let entry = autoballoon_entry > + | lock_entry > > let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ > \t\n][^\n]*)?/ . del /\n/ "\n" ] > let empty = [ label "#empty" . eol ] > diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf > index c104d40..ba3de7a 100644 > --- a/src/libxl/libxl.conf > +++ b/src/libxl/libxl.conf > @@ -10,3 +10,13 @@ > # autoballoon setting. > # > #autoballoon = 1 > + > + > +# In order to prevent accidentally starting two domains that > +# share one writable disk, libvirt offers two approaches for > +# locking files: sanlock and virtlockd. sanlock is an external > +# project which libvirt integrates with via the libvirt-lock-sanlock > +# package. virtlockd is a libvirt implementation that is enabled with > +# "lockd". Accepted values are "sanlock" and "lockd". > +# > +#lock_manager = "lockd" > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index 1b504fa..29498d5 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj) > VIR_FREE(cfg->libDir); > VIR_FREE(cfg->saveDir); > VIR_FREE(cfg->autoDumpDir); > +VIR_FREE(cfg->lockManagerName); > } > > > @@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, >const char *filename) > { > virConfPtr conf = NULL; > +virConfValuePtr p; > int ret = -1; > > /* Check the file is readable before opening it, otherwise > @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, > if (libxlGetAutoballoonConf(cfg, conf) < 0) > goto cleanup; > > +if ((p = virConfGetValue(conf, "lock_manager"))) { > +if (p->type != VIR_CONF_STRING) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", > + _("Unexpected type for 'lock_manager' setting")); > +goto cleanup; > +} > + > +if (VIR_STRDUP(cfg->lockManagerName, p->str) < 0) > +goto cleanup; > +} > + > ret = 0; > > cleanup: > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h > index 5ba1a71..0a1c0db 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -38,6 +38,7 @@ > # include "virobject.h" > # includ
Re: [libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
Thanks Peter, It looks good but I did no deep introspection of the code. Should I rebase my patch [1] on that? I can test your code as well then. [1] https://github.com/paboldin/libvirt/commits/master Pavel On Tue, Apr 28, 2015 at 4:42 PM, Peter Krempa wrote: > On Tue, Apr 28, 2015 at 11:24:37 +0200, Michal Privoznik wrote: > > On 28.04.2015 11:06, Pavel Boldin wrote: > > > Well, actually that seems to be quite a different bug in there. > > > > > > I will start a new thread. > > > > > > In short: migration seems to be broken by commit > > > 1a92c719101e5bfa6fe2b78006ad04c7f075ea28. This is because introduced > job > > > _MODIFY waits while MIGRATION_OUT is finished to change `mirrorState' > > > variable. This deadlocks the libvirt. > > > > Yep, this is known bug. I've told Peter already like two weeks ago. He > > promised to fix it. It would be nice if we can get the fix into the > release. > > There are already patches for the issue: > > http://www.redhat.com/archives/libvir-list/2015-April/msg00724.html > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Add baybysitting for memory hotplug users
On Tue, Apr 28, 2015 at 17:37:42 +0200, Peter Krempa wrote: > Add sanity checks for some unsupported/invalid configs of memory devices. > > Peter Krempa (3): > qemu: conf: Reject memory device if it would exceed configured max > size > qemu: command: Validate that memory devices slot ID is in range > qemu: Validate available slot count for memory devices > > src/conf/domain_conf.c | 11 +++ > src/qemu/qemu_command.c | 19 ++- > src/qemu/qemu_command.h | 1 + > src/qemu/qemu_driver.c | 6 ++ > src/qemu/qemu_hotplug.c | 8 +++- > 5 files changed, 43 insertions(+), 2 deletions(-) ACK series -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] missing php binding to storage volume download , resize and upload
2015-04-24 14:40 GMT+03:00 Michal Privoznik : > Yeah, it's not as actively developed as libvirt. And it not uncommon for > our bindings to not have an API implemented. The best would be to post a > patch to the list and CC Michal Novotny who has done the majority of > work there. Thanks, i'm send patch. It works for me in my limited testing. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru jabber: v...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1] [libvirt-php] add stream support and upload/resize/download for volume
This patch add to libvirt php binding libvirt stream support Also provide resize/upload/download for libvirt volume Signed-off-by: Vasiliy Tolstov --- configure.ac | 3 +- src/libvirt-php.c | 382 +- src/libvirt-php.h | 17 +++ 3 files changed, 395 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index b854153..7099f9d 100644 --- a/configure.ac +++ b/configure.ac @@ -1,7 +1,6 @@ AC_INIT([libvirt-php], [0.5.0], [http://libvirt.org]) -AM_INIT_AUTOMAKE([-Wall -Werror]) +AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability]) AC_CONFIG_FILES([Makefile tools/Makefile src/Makefile tests/Makefile docs/Makefile]) -AM_INIT_AUTOMAKE([-Wno-portability]) AM_MAINTAINER_MODE([enable]) dnl Checks for programs. diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 6d6fa81..0adc4be 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -61,6 +61,7 @@ int le_libvirt_storagepool; int le_libvirt_volume; int le_libvirt_network; int le_libvirt_nodedev; +int le_libvirt_stream; #if LIBVIR_VERSION_NUMBER>=8000 int le_libvirt_snapshot; #endif @@ -90,6 +91,13 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_connect_get_maxvcpus, NULL) PHP_FE(libvirt_connect_get_encrypted, NULL) PHP_FE(libvirt_connect_get_secure, NULL) + /* Stream functions */ + PHP_FE(libvirt_stream_create, NULL) + PHP_FE(libvirt_stream_close, NULL) + PHP_FE(libvirt_stream_abort, NULL) + PHP_FE(libvirt_stream_finish, NULL) + PHP_FE(libvirt_stream_send, NULL) + PHP_FE(libvirt_stream_recv, NULL) /* Domain functions */ PHP_FE(libvirt_domain_new, NULL) PHP_FE(libvirt_domain_new_get_vnc, NULL) @@ -148,7 +156,7 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_domain_get_screen_dimensions, NULL) PHP_FE(libvirt_domain_send_keys, NULL) PHP_FE(libvirt_domain_send_pointer_event, NULL) -PHP_FE(libvirt_domain_update_device, NULL) + PHP_FE(libvirt_domain_update_device, NULL) /* Domain snapshot functions */ PHP_FE(libvirt_domain_has_current_snapshot, NULL) PHP_FE(libvirt_domain_snapshot_create, NULL) @@ -169,6 +177,9 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_storagevolume_create_xml,NULL) PHP_FE(libvirt_storagevolume_create_xml_from,NULL) PHP_FE(libvirt_storagevolume_delete,NULL) + PHP_FE(libvirt_storagevolume_download,NULL) + PHP_FE(libvirt_storagevolume_upload,NULL) + PHP_FE(libvirt_storagevolume_resize,NULL) PHP_FE(libvirt_storagepool_get_uuid_string, NULL) PHP_FE(libvirt_storagepool_get_name, NULL) PHP_FE(libvirt_storagepool_lookup_by_uuid_string, NULL) @@ -370,6 +381,8 @@ char *translate_counter_type(int type) break; case INT_RESOURCE_DOMAIN: return "domain"; break; + case INT_RESOURCE_STREAM: return "stream"; + break; case INT_RESOURCE_NETWORK: return "network"; break; case INT_RESOURCE_NODEDEV: return "node device"; @@ -444,7 +457,7 @@ void free_tokens(tTokenizer t) Private function name: resource_change_counter Since version: 0.4.2 Description:Function to increment (inc = 1) / decrement (inc = 0) the resource pointers count including the memory location - Arguments: @type [int]: type of resource (INT_RESOURCE_x defines where x can be { CONNECTION | DOMAIN | NETWORK | NODEDEV | STORAGEPOOL | VOLUME | SNAPSHOT }) + Arguments: @type [int]: type of resource (INT_RESOURCE_x defines where x can be { CONNECTION | DOMAIN | NETWORK | NODEDEV | STORAGEPOOL | VOLUME | SNAPSHOT | STREAM }) @conn [virConnectPtr]: libvirt connection pointer associated with the resource, NULL for libvirt connection objects @mem [pointer]: Pointer to memory location for the resource. Will be converted to appropriate uint for the arch. @inc [int/bool]: Increment the counter (1 = add memory location) or decrement the counter (0 = remove memory location) from entries. @@ -724,6 +737,18 @@ void free_resource(int type, arch_uint mem TSRMLS_DC) } } + if (type == INT_RESOURCE_STREAM) { + rv = virStreamFree( (virStreamPtr)mem ); + if (rv != 0) { + DPRINTF("%s: virStreamFree(%p) returned %d (%s)\n", __FUNCTION__, (virStreamPtr)mem, rv, LIBVIRT_G (last_error)); + php_error_docref(NULL TSRMLS_CC, E_WARNING,"virStreamFree failed with %i on destructor: %s", rv, LIBVIRT_G (last_erro
[libvirt] [PATCH v1] [libvirt-php] add stream support and upload/resize/download for volume
This patch add to libvirt php binding libvirt stream support Also provide resize/upload/download for libvirt volume Signed-off-by: Vasiliy Tolstov --- configure.ac | 3 +- src/libvirt-php.c | 382 +- src/libvirt-php.h | 17 +++ 3 files changed, 395 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index b854153..7099f9d 100644 --- a/configure.ac +++ b/configure.ac @@ -1,7 +1,6 @@ AC_INIT([libvirt-php], [0.5.0], [http://libvirt.org]) -AM_INIT_AUTOMAKE([-Wall -Werror]) +AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability]) AC_CONFIG_FILES([Makefile tools/Makefile src/Makefile tests/Makefile docs/Makefile]) -AM_INIT_AUTOMAKE([-Wno-portability]) AM_MAINTAINER_MODE([enable]) dnl Checks for programs. diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 6d6fa81..0adc4be 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -61,6 +61,7 @@ int le_libvirt_storagepool; int le_libvirt_volume; int le_libvirt_network; int le_libvirt_nodedev; +int le_libvirt_stream; #if LIBVIR_VERSION_NUMBER>=8000 int le_libvirt_snapshot; #endif @@ -90,6 +91,13 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_connect_get_maxvcpus, NULL) PHP_FE(libvirt_connect_get_encrypted, NULL) PHP_FE(libvirt_connect_get_secure, NULL) + /* Stream functions */ + PHP_FE(libvirt_stream_create, NULL) + PHP_FE(libvirt_stream_close, NULL) + PHP_FE(libvirt_stream_abort, NULL) + PHP_FE(libvirt_stream_finish, NULL) + PHP_FE(libvirt_stream_send, NULL) + PHP_FE(libvirt_stream_recv, NULL) /* Domain functions */ PHP_FE(libvirt_domain_new, NULL) PHP_FE(libvirt_domain_new_get_vnc, NULL) @@ -148,7 +156,7 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_domain_get_screen_dimensions, NULL) PHP_FE(libvirt_domain_send_keys, NULL) PHP_FE(libvirt_domain_send_pointer_event, NULL) -PHP_FE(libvirt_domain_update_device, NULL) + PHP_FE(libvirt_domain_update_device, NULL) /* Domain snapshot functions */ PHP_FE(libvirt_domain_has_current_snapshot, NULL) PHP_FE(libvirt_domain_snapshot_create, NULL) @@ -169,6 +177,9 @@ static zend_function_entry libvirt_functions[] = { PHP_FE(libvirt_storagevolume_create_xml,NULL) PHP_FE(libvirt_storagevolume_create_xml_from,NULL) PHP_FE(libvirt_storagevolume_delete,NULL) + PHP_FE(libvirt_storagevolume_download,NULL) + PHP_FE(libvirt_storagevolume_upload,NULL) + PHP_FE(libvirt_storagevolume_resize,NULL) PHP_FE(libvirt_storagepool_get_uuid_string, NULL) PHP_FE(libvirt_storagepool_get_name, NULL) PHP_FE(libvirt_storagepool_lookup_by_uuid_string, NULL) @@ -370,6 +381,8 @@ char *translate_counter_type(int type) break; case INT_RESOURCE_DOMAIN: return "domain"; break; + case INT_RESOURCE_STREAM: return "stream"; + break; case INT_RESOURCE_NETWORK: return "network"; break; case INT_RESOURCE_NODEDEV: return "node device"; @@ -444,7 +457,7 @@ void free_tokens(tTokenizer t) Private function name: resource_change_counter Since version: 0.4.2 Description:Function to increment (inc = 1) / decrement (inc = 0) the resource pointers count including the memory location - Arguments: @type [int]: type of resource (INT_RESOURCE_x defines where x can be { CONNECTION | DOMAIN | NETWORK | NODEDEV | STORAGEPOOL | VOLUME | SNAPSHOT }) + Arguments: @type [int]: type of resource (INT_RESOURCE_x defines where x can be { CONNECTION | DOMAIN | NETWORK | NODEDEV | STORAGEPOOL | VOLUME | SNAPSHOT | STREAM }) @conn [virConnectPtr]: libvirt connection pointer associated with the resource, NULL for libvirt connection objects @mem [pointer]: Pointer to memory location for the resource. Will be converted to appropriate uint for the arch. @inc [int/bool]: Increment the counter (1 = add memory location) or decrement the counter (0 = remove memory location) from entries. @@ -724,6 +737,18 @@ void free_resource(int type, arch_uint mem TSRMLS_DC) } } + if (type == INT_RESOURCE_STREAM) { + rv = virStreamFree( (virStreamPtr)mem ); + if (rv != 0) { + DPRINTF("%s: virStreamFree(%p) returned %d (%s)\n", __FUNCTION__, (virStreamPtr)mem, rv, LIBVIRT_G (last_error)); + php_error_docref(NULL TSRMLS_CC, E_WARNING,"virStreamFree failed with %i on destructor: %s", rv, LIBVIRT_G (last_erro
[libvirt] ANNOUNCE: libvirt 1.2.13.1 maintenance release
libvirt 1.2.13.1 is now available. This is a maintenance release of libvirt 1.2.13 with additional bugfixes that have accumulated upstream since the initial release. This release can be downloaded at: http://libvirt.org/sources/stable_updates/libvirt-1.2.13.1.tar.gz Changes in this version: * Fix memory leak in virNetSocketNewConnectUNIX * rng: fix port number range validation * qemu: Don't fail to reboot domains with unresponsive agent * vircommand: fix polling in virCommandProcessIO * util: storage: Fix possible crash when source path is NULL * qemu: set macvtap physdevs online when macvtap is set online * util: set MAC address for VF via netlink message to PF+VF# when possible * xend: Remove a couple of unused function prototypes. * qemuDomainShutdownFlags: Set fakeReboot more frequently * nwfilter: Partly initialize driver even for non-privileged users * virNetSocketNewConnectUNIX: Don't unlink(NULL) * sanlock: Use VIR_ERR_RESOURCE_BUSY if sanlock_acquire fails * qemuMigrationPrecreateStorage: Fix debug message * qemu_migration.c: sleep first before checking for migration status. * qemu_driver: check caps after starting block job * qemu_migrate: use nested job when adding NBD to cookie * util: fix removal of callbacks in virCloseCallbacksRun * qemu: fix race between disk mirror fail and cancel * qemu: fix error propagation in qemuMigrationBegin * qemu: fix crash in qemuProcessAutoDestroy * qemu: blockCopy: Pass adjusted bandwidth when called via blockRebase * virsh: blockCopy: Add missing jump on error path * qemu: end the job when try to blockcopy to non-file destination * nodeinfo: Increase the num of CPU thread siblings to a larger value * relaxng: allow : in /dev/disk/by-path names * qemu: Give hint about -noTSX CPU model * build: fix race when creating the cpu_map.xml symlink * Don't validata filesystem target type * Document behavior of compat when creating qcow2 volumes * Fix typo in error message * qemu: change accidental VIR_WARNING back to VIR_DEBUG * conf: fix parsing of NUMA settings in VM status XML * qemu: skip precreation of network disks * qemu: do not overwrite the error in qemuDomainObjExitMonitor * libxl: Don't overwrite errors from xenconfig * util: more verbose error when failing to create macvtap device * qemu: hotplug: Use checker function to check if disk is empty * qemu: driver: Fix cold-update of removable storage devices * qemu: Check for negative port values in network drive configuration * virsh: fix report of non-active commit completion * util: don't fail if no PortData is found while getting migrateData * Clarify the meaning of version in redirdev filters * xenapi: Resolve Coverity REVERSE_INULL * xenapi: Resolve Coverity REVERSE_INULL * xenapi: Resolve Coverity NULL_RETURNS * xenapi: Resolve Coverity NO_EFFECT * xenapi: Resolve Coverity FORWARD_NULL * RNG: Allow multiple parameters to be passed to an interface filter * domain_conf: fix crash in virDomainObjListFindByUUIDInternal * {domain, network}_conf: disable autostart when deleting config * qemu: Remove unnecessary virReportError on networkGetNetworkAddress return * virQEMUCapsInitQMP: Don't dispose locked @vm * qemu: fix memory leak in qemuAgentGetFSInfo * docs: add a note that spice channel is usable only with spice graphics * locking: Fix flags in virLockManagerLockDaemonNew * tests: fix qemuxml2argvtest to be arch independent * tests: Add test for virtio-mmio address type * qemu: Allow spaces in disk serial * storage: tweak condition to properly test lseek * virsh: tweak domif-getlink link state reporting message * qemu: snapshot: Don't skip check for qcow2 format with network disks * networkLookupByUUID: Improve error message * qemuProcessReconnect: Fill in pid file path * tests : Add test for 'ppc64le' architecture. * RNG: Add 'ppc64le' arch and newer pseries-2.* machine types * schema: Fix interface link state schema * conf: De-duplicate scheduling policy enums * qemu: Don't crash in qemuDomainOpenChannel() * virsh.pod: Update find-storage-pool-sources[-as] man page * iscsi: Adjust error message for findStorageSources backend * virsh.pod: Add information regarding LXC for setmem, memtune, and dominfo * docs: add a note that attr 'managed' is only used by PCI devices * Check if domain is running in qemuDomainAgentIsAvailable * Pass virDomainObjPtr to qemuDomainAgentAvailable * Check for qemu guest agent availability after getting the job * storage: fs: Ignore volumes that fail to open with EACCESS/EPERM * domain: conf: Don't validate VM ostype/arch at daemon startup * domain: conf: Better errors on bad os values * spec: Point fedora --with-loader-nvram at nightly firmware repo * configure: Report --with-loader-nvram value in summary * configure: Fix --loader-nvram typo * cpu: Add {Haswell,Broadwell}-noTSX CPU models * domcaps: Check for architecture more wisely * daemon: Clear fake domain def object that is used to check ACL prior to use * util: identity: Harden virIdentitySetCurrent() * qemu: Alw
Re: [libvirt] [PATCH 2/2] parallels: implement domainDetachDevice and domainDetachDeviceFlags
On 04/23/2015 09:37 PM, Maxim Nestratov wrote: New functions utilize previosly added prlsdkDelDisk and prlsdkGetDiskIndex Signed-off-by: Maxim Nestratov --- src/parallels/parallels_driver.c | 71 ++ src/parallels/parallels_sdk.c| 30 src/parallels/parallels_sdk.h|3 ++ 3 files changed, 104 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 07f1311..b667e02 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1098,6 +1098,75 @@ static int parallelsDomainAttachDevice(virDomainPtr dom, const char *xml) VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE); } +static int parallelsDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, +unsigned int flags) +{ +int ret = -1; +parallelsConnPtr privconn = dom->conn->privateData; +virDomainDeviceDefPtr dev = NULL; +virDomainObjPtr privdom = NULL; +bool domactive = false; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +privdom = parallelsDomObjFromDomain(dom); +if (privdom == NULL) +return -1; + +if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device detach needs VIR_DOMAIN_AFFECT_CONFIG " + "flag to be set")); +goto cleanup; +} + +domactive = virDomainObjIsActive(privdom); +if (!domactive && (flags & VIR_DOMAIN_AFFECT_LIVE)) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot do live update a device on " + "inactive domain")); +goto cleanup; +} +if (domactive && !(flags & VIR_DOMAIN_AFFECT_LIVE)) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Updates on a running domain need " + "VIR_DOMAIN_AFFECT_LIVE flag")); +} + +dev = virDomainDeviceDefParse(xml, privdom->def, privconn->caps, + privconn->xmlopt, VIR_DOMAIN_XML_INACTIVE); +if (dev == NULL) +goto cleanup; + +switch (dev->type) { +case VIR_DOMAIN_DEVICE_DISK: +ret = prlsdkDetachVolume(dom->conn, privdom, dev->data.disk); +if (ret) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("disk detach failed")); +goto cleanup; +} +break; +default: +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("device type '%s' cannot be detached"), + virDomainDeviceTypeToString(dev->type)); +break; +} + +ret = 0; + cleanup: +virObjectUnlock(privdom); +return ret; +} + +static int parallelsDomainDetachDevice(virDomainPtr dom, const char *xml) +{ +return parallelsDomainDetachDeviceFlags(dom, xml, +VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_LIVE); +} + static virHypervisorDriver parallelsDriver = { .name = "Parallels", .connectOpen = parallelsConnectOpen,/* 0.10.0 */ @@ -1134,6 +1203,8 @@ static virHypervisorDriver parallelsDriver = { .domainUndefineFlags = parallelsDomainUndefineFlags, /* 1.2.10 */ .domainAttachDevice = parallelsDomainAttachDevice, /* 1.2.15 */ .domainAttachDeviceFlags = parallelsDomainAttachDeviceFlags, /* 1.2.15 */ +.domainDetachDevice = parallelsDomainDetachDevice, /* 1.2.15 */ +.domainDetachDeviceFlags = parallelsDomainDetachDeviceFlags, /* 1.2.15 */ .domainIsActive = parallelsDomainIsActive, /* 1.2.10 */ .connectDomainEventRegisterAny = parallelsConnectDomainEventRegisterAny, /* 1.2.10 */ .connectDomainEventDeregisterAny = parallelsConnectDomainEventDeregisterAny, /* 1.2.10 */ diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 5e6e21c..94274a2 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -3106,6 +3106,36 @@ prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk) return idx; } +int prlsdkDetachVolume(virConnectPtr conn, + virDomainObjPtr dom, + virDomainDiskDefPtr disk) +{ +int ret = -1, idx; +parallelsConnPtr privconn = conn->privateData; +parallelsDomObjPtr privdom = dom->privateData; +PRL_HANDLE job = PRL_INVALID_HANDLE; + +idx = prlsdkGetDiskIndex(privdom->sdkdom, disk); +if (idx < 0) +goto cleanup; + +job = PrlVm_BeginEdit(privdom->sdkdom); +if (PRL_FAILED(waitJob(job, privconn->jobTimeout))) +goto cleanup; + +ret = prlsdkDelDisk(privdom->sdkdom, idx); +if (ret == 0) { +job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); +if (PRL_FAILED(waitJ
Re: [libvirt] [PATCH 1/2] parallels: add prlsdkDelDisk and prlsdkGetDiskIndex functions
On 04/23/2015 09:37 PM, Maxim Nestratov wrote: Signed-off-by: Maxim Nestratov --- src/parallels/parallels_sdk.c | 65 + 1 files changed, 65 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d54f894..5e6e21c 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2844,6 +2844,25 @@ static void prlsdkDelNet(parallelsConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); } +static int prlsdkDelDisk(PRL_HANDLE sdkdom, int idx) +{ +int ret = -1; +PRL_RESULT pret; +PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetHardDisk(sdkdom, idx, &sdkdisk); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_Remove(sdkdisk); +prlsdkCheckRetGoto(pret, cleanup); + +ret = 0; + + cleanup: +PrlHandle_Free(sdkdisk); +return ret; +} + static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk, bool bootDisk) { PRL_RESULT pret; @@ -3042,6 +3061,52 @@ prlsdkAttachVolume(virConnectPtr conn, } static int +prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk) +{ +int idx = -1; +char *buf = NULL; +PRL_UINT32 buflen = 0; +PRL_RESULT pret; +PRL_UINT32 hddCount; +PRL_UINT32 i; +PRL_HANDLE hdd = PRL_INVALID_HANDLE; + +pret = PrlVmCfg_GetHardDisksCount(sdkdom, &hddCount); +prlsdkCheckRetGoto(pret, cleanup); + +for (i = 0; i < hddCount; ++i) { + +pret = PrlVmCfg_GetHardDisk(sdkdom, i, &hdd); +prlsdkCheckRetGoto(pret, cleanup); + +pret = PrlVmDev_GetFriendlyName(hdd, 0, &buflen); +prlsdkCheckRetGoto(pret, cleanup); + +if (VIR_ALLOC_N(buf, buflen) < 0) +goto cleanup; + +pret = PrlVmDev_GetFriendlyName(hdd, buf, &buflen); +prlsdkCheckRetGoto(pret, cleanup); + +if (STRNEQ(disk->src->path, buf)) { + +PrlHandle_Free(hdd); +hdd = PRL_INVALID_HANDLE; +VIR_FREE(buf); +continue; +} + +VIR_FREE(buf); +idx = i; +break; +} + + cleanup: +PrlHandle_Free(hdd); +return idx; +} + +static int prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs) { PRL_RESULT pret; Looks good to me, ACKed and pushed, thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ANNOUNCE: libvirt 1.2.9.3 maintenance release
libvirt 1.2.9.3 is now available. This is a maintenance release of libvirt 1.2.9 with additional bugfixes that have accumulated upstream since the initial release. This release can be downloaded at: http://libvirt.org/sources/stable_updates/libvirt-1.2.9.3.tar.gz Changes in this version: * storage: fs: Ignore volumes that fail to open with EACCESS/EPERM * domain: conf: Don't validate VM ostype/arch at daemon startup * domain: conf: Better errors on bad os values * Report original error when QMP probing fails with new QEMU * cpu: Add {Haswell,Broadwell}-noTSX CPU models * storage: qemu: Fix security labelling of new image chain elements * Ignore CPU features without a model for host-passthrough * Do not format CPU features without a model * domcaps: Check for architecture more wisely * daemon: Clear fake domain def object that is used to check ACL prior to use * util: identity: Harden virIdentitySetCurrent() * qemu: Build nvram directory at driver startup * qemu: Build channel autosocket directory at driver startup * qemu: chown autoDumpPath on driver startup * qemu: conf: Clarify paths that are relative to libDir * avoid using deprecated udev logging functions * qemu: Always refresh capabilities if no found * qemu: move setting emulatorpin ahead of monitor showing up * rpc: Don't unref identity object while callbacks still can be executed * conf: tests: fix virDomainNetDefFormat for vhost-user in client mode * Document that USB hostdevs do not need nodeDettach * Document behavior of compat when creating qcow2 volumes * Clarify the meaning of version in redirdev filters * Strip control codes in virBufferEscapeString * Ignore storage volumes with control codes in their names * Strip control characters from sysfs attributes * Add functions dealing with control characters in strings * virNetworkDefUpdateIPDHCPHost: Don't crash when updating network * daemon: avoid memleak when ListAll returns nothing * conf: error out on missing dhcp host attributes * conf: error out on invalid host id * conf: Don't format actual network definition in migratable XML * conf: Fix libvirtd crash and memory leak caused by virDomainVcpuPinDel() For info about past maintenance releases, see: http://wiki.libvirt.org/page/Maintenance_Releases Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] qemu: blockCopy: Allow reuse of raw image for shallow block copy
The documentation states that for shallow block copy the image has to have the same guest visible content as backing file of the current image if the file is being reused. This condition can be achieved also with a raw file (or a qcow without a backing file) so remove the condition that would disallow it. (This patch additionally fixes crash described in https://bugzilla.redhat.com/show_bug.cgi?id=1215569 ) --- V2: - different approach to fix Eric's comment src/qemu/qemu_driver.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 80463f2..581211a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17122,11 +17122,17 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) goto endjob; +/* clear the _SHALLOW flag if there is only one layer */ +if (!disk->src->backingStore) +flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW; + +/* unless the user provides a pre-created file, shallow copy into a raw + * file is not possible */ if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) && -mirror->format == VIR_STORAGE_FILE_RAW && -disk->src->backingStore->path) { +!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT) && +mirror->format == VIR_STORAGE_FILE_RAW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' has backing file, so raw shallow copy " + _("shallow copy of disk '%s' into a raw file " "is not possible"), disk->dst); goto endjob; -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Fix grammar in comments.
Replace all occurrences of "stream write to differences to" with "stream to write differences to". --- tests/testutils.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 6a8fe6a..89026c6 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -458,7 +458,7 @@ virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, /** - * @param stream: output stream write to differences to + * @param stream: output stream to write differences to * @param expect: expected output text * @param expectName: name designator of the expected text * @param actual: actual output text @@ -532,7 +532,7 @@ int virtTestDifferenceFull(FILE *stream, } /** - * @param stream: output stream write to differences to + * @param stream: output stream to write differences to * @param expect: expected output text * @param actual: actual output text * @@ -548,7 +548,7 @@ int virtTestDifference(FILE *stream, /** - * @param stream: output stream write to differences to + * @param stream: output stream to write differences to * @param expect: expected output text * @param actual: actual output text * -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: command: Validate that memory devices slot ID is in range
slot id, if specified, has to be less than the slots count. --- src/qemu/qemu_command.c | 11 ++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba15dc9..21daf18 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4946,6 +4946,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, char * qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4972,6 +4973,14 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, return NULL; } +if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM && +mem->info.addr.dimm.slot >= def->mem.memory_slots) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device slot '%u' exceeds slots count '%u'"), + mem->info.addr.dimm.slot, def->mem.memory_slots); +return NULL; +} + virBufferAsprintf(&buf, "pc-dimm,node=%d,memdev=mem%s,id=%s", mem->targetNode, mem->info.alias, mem->info.alias); @@ -8821,7 +8830,7 @@ qemuBuildCommandLine(virConnectPtr conn, qemuCaps, cfg))) goto error; -if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], qemuCaps))) { +if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, qemuCaps))) { VIR_FREE(backStr); goto error; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a29db41..675eb62 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -175,6 +175,7 @@ int qemuBuildMemoryBackendStr(unsigned long long size, bool force); char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + virDomainDefPtr def, virQEMUCapsPtr qemuCaps); /* Legacy, pre device support */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 58224bf..ba92320 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1736,7 +1736,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0) goto cleanup; -if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv->qemuCaps))) +if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps))) goto cleanup; qemuDomainMemoryDeviceAlignSize(mem); -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] qemu: Add baybysitting for memory hotplug users
Add sanity checks for some unsupported/invalid configs of memory devices. Peter Krempa (3): qemu: conf: Reject memory device if it would exceed configured max size qemu: command: Validate that memory devices slot ID is in range qemu: Validate available slot count for memory devices src/conf/domain_conf.c | 11 +++ src/qemu/qemu_command.c | 19 ++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_driver.c | 6 ++ src/qemu/qemu_hotplug.c | 8 +++- 5 files changed, 43 insertions(+), 2 deletions(-) -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: conf: Reject memory device if it would exceed configured max size
If the added memory device would exceed the maximum memory size, reject it. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1216046 --- src/conf/domain_conf.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fc48ed5..7e4f0af 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21661,6 +21661,17 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, return -1; } +if (dev->type == VIR_DOMAIN_DEVICE_MEMORY) { +unsigned long long sz = dev->data.memory->size; + +if ((virDomainDefGetMemoryActual(def) + sz) > def->mem.max_memory) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Attaching memory device with size '%llu' would " + "exceed domain's maxMemory config"), sz); +return -1; +} +} + return 0; } -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: Validate available slot count for memory devices
While qemu would reject the configuration we can check whether it makes sense to plug the device upfront. --- src/qemu/qemu_command.c | 8 src/qemu/qemu_driver.c | 6 ++ src/qemu/qemu_hotplug.c | 6 ++ 3 files changed, 20 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 21daf18..e62833f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8822,6 +8822,14 @@ qemuBuildCommandLine(virConnectPtr conn, /* memory hotplug requires NUMA to be enabled - we already checked * that memory devices are present only when NUMA is */ + +if (def->nmems > def->mem.memory_slots) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device count '%zu' exceeds slots count '%u'"), + def->nmems, def->mem.memory_slots); +goto error; +} + for (i = 0; i < def->nmems; i++) { char *backStr; char *dimmStr; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 80463f2..d181439 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8489,6 +8489,12 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, break; case VIR_DOMAIN_DEVICE_MEMORY: +if (vmdef->nmems == vmdef->mem.memory_slots) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no free memory device slot available")); +return -1; +} + if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) return -1; dev->data.memory = NULL; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ba92320..613b728 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1730,6 +1730,12 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, int id; int ret = -1; +if (vm->def->nmems == vm->def->mem.memory_slots) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no free memory device slot available")); +goto cleanup; +} + if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems) < 0) goto cleanup; -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] tests: fix memleak in qemumonitorjsontest
On 28.04.2015 03:16, Zhang Bo wrote: > the free callback should be qemuMonitorChardevInfoFree rather than just 'free' > when virHashCreate'ing the chardevInfo hash. > > Zhang Bo (2): > qemu: make qemuMonitorChardevInfoFree non-static > tests: free ChardevInfo correctly in qemumonitorjsontest > > src/qemu/qemu_monitor.c | 2 +- > src/qemu/qemu_monitor.h | 1 + > tests/qemumonitorjsontest.c | 4 ++-- > 3 files changed, 4 insertions(+), 3 deletions(-) > ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix building virnetserverclientmock with MinGW
On 28.04.2015 13:15, Martin Kletzander wrote: > Signed-off-by: Pavel Fedin > Signed-off-by: Martin Kletzander > --- > > Notes: > This was a part of commit 9dc57ce2, but separable; plus the library > should be in LIBADD and not LDFLAGS. Depends on my previous cleanup [2] > > [1] https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9dc57ce2 > [2] https://www.redhat.com/archives/libvir-list/2015-April/msg01336.html > > v2: Use LIBADD instead of LDADDS (which doesn't even exist) > > tests/Makefile.am | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index dc7daaa..8e2dbec 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -931,6 +931,7 @@ virnetserverclientmock_la_SOURCES = \ > virnetserverclientmock.c > virnetserverclientmock_la_CFLAGS = $(AM_CFLAGS) > virnetserverclientmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) > +virnetserverclientmock_la_LIBADD = $(GNULIB_LIBS) > > if WITH_GNUTLS > virnettlscontexttest_SOURCES = \ > ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-cim][PATCH] list_util.h: Drop inline modifiers
On 04/28/2015 07:58 AM, Michal Privoznik wrote: > There's no need to mark a function as inline in the function > declaration. In fact, it causes a compilation error: > > CC xmlgen.lo > In file included from acl_parsing.h:29:0, > from xmlgen.h:26, > from capability_parsing.c:37: > list_util.h:67:21: error: inline function ‘list_node_prev_node’ declared but > never defined [-Werror] > inline list_node_t *list_node_prev_node(list_node_t *node); > ^ > list_util.h:66:14: error: inline function ‘list_node_prev’ declared but never > defined [-Werror] > inline void*list_node_prev(list_node_t *node); > ^ > list_util.h:64:21: error: inline function ‘list_node_next_node’ declared but > never defined [-Werror] > inline list_node_t *list_node_next_node(list_node_t *node); > ^ > list_util.h:63:14: error: inline function ‘list_node_next’ declared but never > defined [-Werror] > inline void*list_node_next(list_node_t *node); > ^ > list_util.h:61:21: error: inline function ‘list_last_node’ declared but never > defined [-Werror] > inline list_node_t *list_last_node(list_t *list); > ^ > list_util.h:60:14: error: inline function ‘list_last’ declared but never > defined [-Werror] > inline void*list_last(list_t *list); > ^ > list_util.h:58:21: error: inline function ‘list_first_node’ declared but > never defined [-Werror] > inline list_node_t *list_first_node(list_t *list); > ^ > list_util.h:57:14: error: inline function ‘list_first’ declared but never > defined [-Werror] > inline void*list_first(list_t *list); > ^ > list_util.h:55:13: error: inline function ‘list_node_data_set’ declared but > never defined [-Werror] > inline void list_node_data_set(list_node_t *node, void *data); > ^ > list_util.h:54:14: error: inline function ‘list_node_data_get’ declared but > never defined [-Werror] > inline void *list_node_data_get(list_node_t *node); > ^ > list_util.h:52:21: error: inline function ‘list_count’ declared but never > defined [-Werror] > inline unsigned int list_count(list_t *list); > ^ > cc1: all warnings being treated as errors > Makefile:499: recipe for target 'capability_parsing.lo' failed > make[2]: *** [capability_parsing.lo] Error 1 > make[2]: *** Waiting for unfinished jobs > > Signed-off-by: Michal Privoznik > --- > libxkutil/list_util.h | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > So back to the original patch - it's a build breaker, so I suppose it could be pushed for that reason, but to be official... ACK, John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virtual smartcard support for GPG backend?
On 2015-04-26 13:28, r...@openmailbox.org wrote: Hi. I am trying to get a virtual smartcard attached to a vm but I want it to use GPG instead of NSS. RedHat focuses on NSS becuase of PKCS#11 requirements and FIPS approval, but for most of the community its GPG that matters for smartcards. Is is possible to use GPG on the host instead of NSS with virtual smartcards? Please document how or add support for it. Is using a virtual smartcard make the host less secure from a rogue vm? If there are bugs in GPG/NSS backend on the host can they be abused by untrusted code in the vm? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Is it the wrong place to ask? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
On Tue, Apr 28, 2015 at 11:24:37 +0200, Michal Privoznik wrote: > On 28.04.2015 11:06, Pavel Boldin wrote: > > Well, actually that seems to be quite a different bug in there. > > > > I will start a new thread. > > > > In short: migration seems to be broken by commit > > 1a92c719101e5bfa6fe2b78006ad04c7f075ea28. This is because introduced job > > _MODIFY waits while MIGRATION_OUT is finished to change `mirrorState' > > variable. This deadlocks the libvirt. > > Yep, this is known bug. I've told Peter already like two weeks ago. He > promised to fix it. It would be nice if we can get the fix into the release. There are already patches for the issue: http://www.redhat.com/archives/libvir-list/2015-April/msg00724.html signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: fs: Ignore volumes that fail to open with EPERM
On 04/28/2015 07:50 AM, JĂ¡n Tomko wrote: > The summary says 'EPERM', but the code checks for EACCES. > > On Mon, Apr 27, 2015 at 11:57:53AM -0400, Cole Robinson wrote: >> Trying to use qemu:///session to create a storage pool pointing at >> /tmp will usually fail with something like: >> >> $ virsh pool-start tmp >> error: Failed to start pool tmp >> error: cannot open volume >> '/tmp/systemd-private-c38cf0418d7a4734a66a8175996c384f-colord.service-kEyiTA': >> Permission denied >> >> If any volume in an FS pool can't be opened by the daemon, the refresh >> fails, and the pool can't be used. >> >> This causes pain for virt-install/virt-manager though. Imaging a user >> downloads a disk image to /tmp. virt-manager wants to import /tmp as >> a storage pool, so we can detect what disk format it is, and set the >> XML correctly. However this case will likely fail as explained above. >> >> Change the logic here to skip volumes that fail to open. This could >> conceivably cause user complaints along the lines of 'why doesn't >> libvirt show $ROOT-OWNED-VOLUME-FOO', but figuring that currently >> the pool won't even startup, I don't think there are any current >> users that care about that case. >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1103308 >> --- >> src/storage/storage_backend.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c >> index e0311e1..186013c 100644 >> --- a/src/storage/storage_backend.c >> +++ b/src/storage/storage_backend.c >> @@ -1445,6 +1445,10 @@ virStorageBackendVolOpen(const char *path, struct >> stat *sb, >> VIR_WARN("ignoring missing fifo '%s'", path); >> return -2; >> } >> +if (errno == EACCES && noerror) { > > Looking at open(2) we are unlikely to hit EPERM, but maybe it would be > better to look for both? > >> +VIR_WARN("ignore permission error for '%s'", path); > > s/ignore/ignoring/ to match the other warnings > > ACK whether you check EPERM or not, as long as you tweak the commit > summary. > Hmm, I got my error codes confused :) I added the EPERM check, fixed the error message and the commit message, and pushed Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] storage: Fix a few issues with pool state tracking
On 04/28/2015 08:07 AM, JĂ¡n Tomko wrote: > On Mon, Apr 27, 2015 at 10:44:54AM -0400, Cole Robinson wrote: >> First patch fixes a regression introduced with pool state patches >> Last 3 patches help fix other issues with the state tracking >> >> Cole Robinson (4): >> storage: Fix autostart dir for qemu:///session >> storage: Don't leave stale state file if pool startup fails >> storage: Break out storageDriverLoadPoolState >> storage: If driver startup state syncing fails, delete statefile >> >> src/storage/storage_driver.c | 109 >> ++- >> 1 file changed, 67 insertions(+), 42 deletions(-) >> > > ACK series > Thanks, pushed now - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-cim][PATCH] list_util.h: Drop inline modifiers
On Tue, Apr 28, 2015 at 08:53:28AM -0400, John Ferlan wrote: > > > On 04/28/2015 07:58 AM, Michal Privoznik wrote: > > There's no need to mark a function as inline in the function > > declaration. In fact, it causes a compilation error: > > > > CC xmlgen.lo > > In file included from acl_parsing.h:29:0, > > from xmlgen.h:26, > > from capability_parsing.c:37: > > list_util.h:67:21: error: inline function ‘list_node_prev_node’ declared > > but never defined [-Werror] > > inline list_node_t *list_node_prev_node(list_node_t *node); > > ^ [...] > > cc1: all warnings being treated as errors > > Makefile:499: recipe for target 'capability_parsing.lo' failed > > make[2]: *** [capability_parsing.lo] Error 1 > > make[2]: *** Waiting for unfinished jobs > > > > Signed-off-by: Michal Privoznik > > --- > > libxkutil/list_util.h | 22 +++--- > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > The last time I compiled libvirt-cim this wasn't an issue! Additionally > my daily Coverity builds libvirt-cim from upstream from scratch without > issue... > > So is this from a 'newer' compiler? I have: > > $ gcc --version > gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6) > Copyright (C) 2014 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > I'm not disagreeing this is a problem and needs to be fixed... > > John > This happens only on fedora-rawhide where is a new gcc that almost fully supports the new c99 standard and that standard is a default one on that gcc version. $ gcc --version gcc (GCC) 5.0.0 20150407 (Red Hat 5.0.0-0.22) > FWIW: > > libvirt-cim patches have been sent to the libvirt-cim list > @libvirt-...@redhat.com and not libvir-list... > > > diff --git a/libxkutil/list_util.h b/libxkutil/list_util.h > > index 6510272..6582dfe 100644 > > --- a/libxkutil/list_util.h > > +++ b/libxkutil/list_util.h > > @@ -49,22 +49,22 @@ void list_remove_node(list_t *list, list_node_t *node); > > > > bool list_foreach(list_t *list, list_foreach_cb cb, void *user_data); > > > > -inline unsigned int list_count(list_t *list); > > +unsigned int list_count(list_t *list); > > > > -inline void *list_node_data_get(list_node_t *node); > > -inline void list_node_data_set(list_node_t *node, void *data); > > +void *list_node_data_get(list_node_t *node); > > +void list_node_data_set(list_node_t *node, void *data); > > > > -inline void*list_first(list_t *list); > > -inline list_node_t *list_first_node(list_t *list); > > +void*list_first(list_t *list); > > +list_node_t *list_first_node(list_t *list); > > > > -inline void*list_last(list_t *list); > > -inline list_node_t *list_last_node(list_t *list); > > +void*list_last(list_t *list); > > +list_node_t *list_last_node(list_t *list); > > > > -inline void*list_node_next(list_node_t *node); > > -inline list_node_t *list_node_next_node(list_node_t *node); > > +void*list_node_next(list_node_t *node); > > +list_node_t *list_node_next_node(list_node_t *node); > > > > -inline void*list_node_prev(list_node_t *node); > > -inline list_node_t *list_node_prev_node(list_node_t *node); > > +void*list_node_prev(list_node_t *node); > > +list_node_t *list_node_prev_node(list_node_t *node); > > > > #ifdef __cplusplus > > } /* extern "C" */ > > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] storage: fs: Only force directory permissions if required
On Mon, Apr 27, 2015 at 16:48:44 -0400, Cole Robinson wrote: > Only set directory permissions at pool build time, if: > > - User explicitly requested a mode via the XML > - The directory needs to be created > - We need to do the crazy NFS root-squash workaround > > This allows qemu:///session to call build on an existing directory > like /tmp. > --- > src/storage/storage_backend_fs.c | 16 +++- > src/util/virfile.c | 2 +- > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/src/storage/storage_backend_fs.c > b/src/storage/storage_backend_fs.c > index 344ee4c..e11c240 100644 > --- a/src/storage/storage_backend_fs.c > +++ b/src/storage/storage_backend_fs.c > @@ -769,6 +769,8 @@ virStorageBackendFileSystemBuild(virConnectPtr conn > ATTRIBUTE_UNUSED, > int err, ret = -1; > char *parent = NULL; > char *p = NULL; > +mode_t mode; > +bool needs_create_as_uid; > > virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | >VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); > @@ -802,19 +804,23 @@ virStorageBackendFileSystemBuild(virConnectPtr conn > ATTRIBUTE_UNUSED, > } > } > > +needs_create_as_uid = (pool->def->type == VIR_STORAGE_POOL_NETFS); > +mode = pool->def->target.perms.mode; > +if (mode == (mode_t) -1 && > +(needs_create_as_uid || !virFileExists(pool->def->target.path))) > +mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; > + > /* Now create the final dir in the path with the uid/gid/mode > * requested in the config. If the dir already exists, just set > * the perms. */ > if ((err = virDirCreate(pool->def->target.path, > -(pool->def->target.perms.mode == (mode_t) -1 ? > - VIR_STORAGE_DEFAULT_POOL_PERM_MODE : > - pool->def->target.perms.mode), > +mode, > pool->def->target.perms.uid, > pool->def->target.perms.gid, > VIR_DIR_CREATE_FORCE_PERMS | > VIR_DIR_CREATE_ALLOW_EXIST | > -(pool->def->type == VIR_STORAGE_POOL_NETFS > -? VIR_DIR_CREATE_AS_UID : 0))) < 0) { > +(needs_create_as_uid ? VIR_DIR_CREATE_AS_UID : 0) > +)) < 0) { Closing parentheses on a separate line are ugly. I'd rather see violating the 80 cols rule rather than damaging readability of the code. > goto error; > } > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index 676e7b4..7e49f39 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -2311,7 +2311,7 @@ virDirCreateNoFork(const char *path, > path, (unsigned int) uid, (unsigned int) gid); > goto error; > } > -if ((flags & VIR_DIR_CREATE_FORCE_PERMS) > +if (((flags & VIR_DIR_CREATE_FORCE_PERMS) && mode != (mode_t) -1) This is a usage error of the function. Using mode == -1 and requesting it to be set doesn't make much sense. > && (chmod(path, mode) < 0)) { > ret = -errno; > virReportSystemError(errno, ACK. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] storage: fs: Don't try to chown directory unless user requested
On Mon, Apr 27, 2015 at 16:48:42 -0400, Cole Robinson wrote: > Currently we try to chown any directory passed to virDirCreate, > even if the user didn't request any explicit owner/group via the > pool/vol XML. > > This causes issues with qemu:///session: try to build a pool of > a root owned directory like /tmp, and it fails trying to chown the > directory to the session user. Instead it should just leave things > as they are, unless the user requests changing permissions via > the pool XML. > > Similarly this is annoying if creating a storage pool via system > libvirtd of an existing directory in user $HOME, it's now owned > by root. > > The virDirCreate function is pretty convoluted, since it needs to > fork off in certain specific cases. Try to document that, to make > it clear where exactly we are changing behavior. > --- > src/util/virfile.c | 32 +++- > 1 file changed, 23 insertions(+), 9 deletions(-) > ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] storage: conf: Don't set any default in the XML
On Mon, Apr 27, 2015 at 16:48:43 -0400, Cole Robinson wrote: > The XML parser sets a default if none is explicitly passed in. > This is then used at pool/vol creation time, and unconditionally reported > in the XML. > > The problem with this approach is that it's impossible for other code > to determine if the user explicitly requested a storage mode. There > are some cases where we want to make this distinction, but we currently > can't. > > Handle parsing like we handle /: if no value is > passed in, set it to -1, and adjust the internal consumers to handle > it. > --- > docs/schemas/storagecommon.rng | 5 ++- > src/conf/storage_conf.c| 42 > +++--- > src/storage/storage_backend.c | 20 --- > src/storage/storage_backend.h | 3 ++ > src/storage/storage_backend_fs.c | 9 +++-- > src/storage/storage_backend_logical.c | 4 ++- > tests/storagepoolxml2xmlin/pool-dir.xml| 2 +- > tests/storagepoolxml2xmlout/pool-dir.xml | 2 +- > tests/storagepoolxml2xmlout/pool-netfs-gluster.xml | 2 +- > tests/storagevolxml2xmlin/vol-file.xml | 6 ++-- > tests/storagevolxml2xmlout/vol-file.xml| 6 ++-- > tests/storagevolxml2xmlout/vol-gluster-dir.xml | 2 +- > tests/storagevolxml2xmlout/vol-sheepdog.xml| 2 +- > 13 files changed, 64 insertions(+), 41 deletions(-) > > diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng > index 5f71b10..e4e8a51 100644 > --- a/docs/schemas/storagecommon.rng > +++ b/docs/schemas/storagecommon.rng > @@ -99,7 +99,10 @@ > > > > - > + > + > + -1 > + I'd rather make the mode optional if you want to keep the default value. > > > > diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c > index 4852dfb..7131242 100644 > --- a/src/conf/storage_conf.c > +++ b/src/conf/storage_conf.c > @@ -50,9 +50,6 @@ > > VIR_LOG_INIT("conf.storage_conf"); > > -#define DEFAULT_POOL_PERM_MODE 0755 > -#define DEFAULT_VOL_PERM_MODE 0600 > - > VIR_ENUM_IMPL(virStorageVol, >VIR_STORAGE_VOL_LAST, >"file", "block", "dir", "network", "netdir") > @@ -718,8 +715,7 @@ virStoragePoolDefParseSourceString(const char *srcSpec, > static int > virStorageDefParsePerms(xmlXPathContextPtr ctxt, > virStoragePermsPtr perms, > -const char *permxpath, > -int defaultmode) > +const char *permxpath) > { > char *mode; > long long val; > @@ -730,7 +726,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, > node = virXPathNode(permxpath, ctxt); > if (node == NULL) { > /* Set default values if there is not element */ > -perms->mode = defaultmode; > +perms->mode = (mode_t) -1; > perms->uid = (uid_t) -1; > perms->gid = (gid_t) -1; > perms->label = NULL; > @@ -740,13 +736,12 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, > relnode = ctxt->node; > ctxt->node = node; > > -mode = virXPathString("string(./mode)", ctxt); > -if (!mode) { > -perms->mode = defaultmode; > -} else { > +if ((mode = virXPathString("string(./mode)", ctxt))) { > int tmp; > > -if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || (tmp & ~0777)) { > +if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || > +((tmp & ~0777) && > + tmp != -1)) { > VIR_FREE(mode); > virReportError(VIR_ERR_XML_ERROR, "%s", > _("malformed octal mode")); > @@ -754,6 +749,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, > } > perms->mode = tmp; > VIR_FREE(mode); > +} else { > +perms->mode = (mode_t) -1; > } > > if (virXPathNode("./owner", ctxt) == NULL) { > @@ -947,8 +944,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) > goto error; > > if (virStorageDefParsePerms(ctxt, &ret->target.perms, > -"./target/permissions", > -DEFAULT_POOL_PERM_MODE) < 0) > +"./target/permissions") < 0) > goto error; > } > > @@ -1185,8 +1181,11 @@ virStoragePoolDefFormatBuf(virBufferPtr buf, > > virBufferAddLit(buf, "\n"); > virBufferAdjustIndent(buf, 2); > -virBufferAsprintf(buf, "0%o\n", > - def->target.perms.mode); > +if (def->target.perms.mode == (mode_t) -1) > +virBufferAddLit(buf, "-1\n"); And I'd skip formatting it. > +else > +virBufferAsprintf(buf, "0%o\n", > + def->target.perms
[libvirt] Close libvirt-cim mailing list ? (Was Re: [libvirt-cim][PATCH] list_util.h: Drop inline modifiers)
On Tue, Apr 28, 2015 at 08:53:28AM -0400, John Ferlan wrote: > FWIW: > > libvirt-cim patches have been sent to the libvirt-cim list > @libvirt-...@redhat.com and not libvir-list... Given that the libvirt CIM project is in maint mode only aat this point, with little to no feature work, I'm not sure there is a compelling reason to continue with a separate mailing list for it. I'd suggest we close down libvirt-...@redhat.com and redirect all discussion & patches to this main libvir-list@redhat.com list, alongside all other libvirt projects. 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] [libvirt-cim][PATCH] list_util.h: Drop inline modifiers
On 28.04.2015 14:53, John Ferlan wrote: > > > On 04/28/2015 07:58 AM, Michal Privoznik wrote: >> There's no need to mark a function as inline in the function >> declaration. In fact, it causes a compilation error: >> >> CC xmlgen.lo >> In file included from acl_parsing.h:29:0, >> from xmlgen.h:26, >> from capability_parsing.c:37: >> list_util.h:67:21: error: inline function ‘list_node_prev_node’ declared but >> never defined [-Werror] >> inline list_node_t *list_node_prev_node(list_node_t *node); >> ^ >> list_util.h:66:14: error: inline function ‘list_node_prev’ declared but >> never defined [-Werror] >> inline void*list_node_prev(list_node_t *node); >> ^ >> list_util.h:64:21: error: inline function ‘list_node_next_node’ declared but >> never defined [-Werror] >> inline list_node_t *list_node_next_node(list_node_t *node); >> ^ >> list_util.h:63:14: error: inline function ‘list_node_next’ declared but >> never defined [-Werror] >> inline void*list_node_next(list_node_t *node); >> ^ >> list_util.h:61:21: error: inline function ‘list_last_node’ declared but >> never defined [-Werror] >> inline list_node_t *list_last_node(list_t *list); >> ^ >> list_util.h:60:14: error: inline function ‘list_last’ declared but never >> defined [-Werror] >> inline void*list_last(list_t *list); >> ^ >> list_util.h:58:21: error: inline function ‘list_first_node’ declared but >> never defined [-Werror] >> inline list_node_t *list_first_node(list_t *list); >> ^ >> list_util.h:57:14: error: inline function ‘list_first’ declared but never >> defined [-Werror] >> inline void*list_first(list_t *list); >> ^ >> list_util.h:55:13: error: inline function ‘list_node_data_set’ declared but >> never defined [-Werror] >> inline void list_node_data_set(list_node_t *node, void *data); >> ^ >> list_util.h:54:14: error: inline function ‘list_node_data_get’ declared but >> never defined [-Werror] >> inline void *list_node_data_get(list_node_t *node); >> ^ >> list_util.h:52:21: error: inline function ‘list_count’ declared but never >> defined [-Werror] >> inline unsigned int list_count(list_t *list); >> ^ >> cc1: all warnings being treated as errors >> Makefile:499: recipe for target 'capability_parsing.lo' failed >> make[2]: *** [capability_parsing.lo] Error 1 >> make[2]: *** Waiting for unfinished jobs >> >> Signed-off-by: Michal Privoznik >> --- >> libxkutil/list_util.h | 22 +++--- >> 1 file changed, 11 insertions(+), 11 deletions(-) >> > > The last time I compiled libvirt-cim this wasn't an issue! Additionally > my daily Coverity builds libvirt-cim from upstream from scratch without > issue... > > So is this from a 'newer' compiler? I have: > > $ gcc --version > gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6) > Copyright (C) 2014 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > I'm not disagreeing this is a problem and needs to be fixed... $ gcc --version gcc (GCC) 5.1.1 20150422 (Red Hat 5.1.1-1) Copyright (C) 2015 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. So yes, I believe that's the problem. > > John > > FWIW: > > libvirt-cim patches have been sent to the libvirt-cim list > @libvirt-...@redhat.com and not libvir-list... Ah, sorry about that. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-cim][PATCH] list_util.h: Drop inline modifiers
On 04/28/2015 07:58 AM, Michal Privoznik wrote: > There's no need to mark a function as inline in the function > declaration. In fact, it causes a compilation error: > > CC xmlgen.lo > In file included from acl_parsing.h:29:0, > from xmlgen.h:26, > from capability_parsing.c:37: > list_util.h:67:21: error: inline function ‘list_node_prev_node’ declared but > never defined [-Werror] > inline list_node_t *list_node_prev_node(list_node_t *node); > ^ > list_util.h:66:14: error: inline function ‘list_node_prev’ declared but never > defined [-Werror] > inline void*list_node_prev(list_node_t *node); > ^ > list_util.h:64:21: error: inline function ‘list_node_next_node’ declared but > never defined [-Werror] > inline list_node_t *list_node_next_node(list_node_t *node); > ^ > list_util.h:63:14: error: inline function ‘list_node_next’ declared but never > defined [-Werror] > inline void*list_node_next(list_node_t *node); > ^ > list_util.h:61:21: error: inline function ‘list_last_node’ declared but never > defined [-Werror] > inline list_node_t *list_last_node(list_t *list); > ^ > list_util.h:60:14: error: inline function ‘list_last’ declared but never > defined [-Werror] > inline void*list_last(list_t *list); > ^ > list_util.h:58:21: error: inline function ‘list_first_node’ declared but > never defined [-Werror] > inline list_node_t *list_first_node(list_t *list); > ^ > list_util.h:57:14: error: inline function ‘list_first’ declared but never > defined [-Werror] > inline void*list_first(list_t *list); > ^ > list_util.h:55:13: error: inline function ‘list_node_data_set’ declared but > never defined [-Werror] > inline void list_node_data_set(list_node_t *node, void *data); > ^ > list_util.h:54:14: error: inline function ‘list_node_data_get’ declared but > never defined [-Werror] > inline void *list_node_data_get(list_node_t *node); > ^ > list_util.h:52:21: error: inline function ‘list_count’ declared but never > defined [-Werror] > inline unsigned int list_count(list_t *list); > ^ > cc1: all warnings being treated as errors > Makefile:499: recipe for target 'capability_parsing.lo' failed > make[2]: *** [capability_parsing.lo] Error 1 > make[2]: *** Waiting for unfinished jobs > > Signed-off-by: Michal Privoznik > --- > libxkutil/list_util.h | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > The last time I compiled libvirt-cim this wasn't an issue! Additionally my daily Coverity builds libvirt-cim from upstream from scratch without issue... So is this from a 'newer' compiler? I have: $ gcc --version gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6) Copyright (C) 2014 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. I'm not disagreeing this is a problem and needs to be fixed... John FWIW: libvirt-cim patches have been sent to the libvirt-cim list @libvirt-...@redhat.com and not libvir-list... > diff --git a/libxkutil/list_util.h b/libxkutil/list_util.h > index 6510272..6582dfe 100644 > --- a/libxkutil/list_util.h > +++ b/libxkutil/list_util.h > @@ -49,22 +49,22 @@ void list_remove_node(list_t *list, list_node_t *node); > > bool list_foreach(list_t *list, list_foreach_cb cb, void *user_data); > > -inline unsigned int list_count(list_t *list); > +unsigned int list_count(list_t *list); > > -inline void *list_node_data_get(list_node_t *node); > -inline void list_node_data_set(list_node_t *node, void *data); > +void *list_node_data_get(list_node_t *node); > +void list_node_data_set(list_node_t *node, void *data); > > -inline void*list_first(list_t *list); > -inline list_node_t *list_first_node(list_t *list); > +void*list_first(list_t *list); > +list_node_t *list_first_node(list_t *list); > > -inline void*list_last(list_t *list); > -inline list_node_t *list_last_node(list_t *list); > +void*list_last(list_t *list); > +list_node_t *list_last_node(list_t *list); > > -inline void*list_node_next(list_node_t *node); > -inline list_node_t *list_node_next_node(list_node_t *node); > +void*list_node_next(list_node_t *node); > +list_node_t *list_node_next_node(list_node_t *node); > > -inline void*list_node_prev(list_node_t *node); > -inline list_node_t *list_node_prev_node(list_node_t *node); > +void*list_node_prev(list_node_t *node); > +list_node_t *list_node_prev_node(list_node_t *node); > > #ifdef __cplusplus > } /* extern "C" */ > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repo link for libvirt-cim throws 404
On 04/28/2015 08:23 AM, Nehal J Wani wrote: > The repository link for libvirt-cim at > http://libvirt.org/CIM/downloads.html seems outdated. Shouldn't it be: > git clone git://libvirt.org/libvirt-cim > instead of > hg clone http://libvirt.org/hg/libvirt-cim > yes, those instructions are really out of date, but then again not much activity is taking place in libvirt-cim, so the obscurity hasn't exactly caused any grief. The last patches reviewed/pushed were almost a year ago. The last upstream release was 0.6.3 on 7/25/13. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/3] IOThread algorithm followups
On Tue, Apr 28, 2015 at 07:23:02 -0400, John Ferlan wrote: > > > On 04/28/2015 06:40 AM, John Ferlan wrote: > > v1 here: > > http://www.redhat.com/archives/libvir-list/2015-April/msg01382.html > > > > v2 changes: > > > > Patch 1 - Add a !STRPREFIX(tmp, "iothread") of the returned iothreads > > to ignore anything that doesn't start with "iothread". > > > > Patch 2 - already ACKed, just difficult to extract. > > > > Patch 3 - NEW: remove qemuMonitorIOThreadInfoFree as requested from > > review of patch 1. > > ... > Thanks for the reviews - now pushed. > > John > > FYI: This cover letter touched on why patch 2 was a repeat... It was > just easier to not extract it - perhaps the git merge would have worked, > perhaps it would not have. I've tried it just now and both interactive rebase and cherry-pick didn't have problem to move commit 2 to the beginning of the series. The patches don't share any context. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Repo link for libvirt-cim throws 404
The repository link for libvirt-cim at http://libvirt.org/CIM/downloads.html seems outdated. Shouldn't it be: git clone git://libvirt.org/libvirt-cim instead of hg clone http://libvirt.org/hg/libvirt-cim -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/6] Add vmport feature
On Tue, Apr 28, 2015 at 11:28:38AM +0200, Marc-André Lureau wrote: Hi Martin On Mon, Apr 27, 2015 at 4:20 PM, Martin Kletzander wrote: ACK series with two things to consider mentioned in-line (not hard requirements, but it would be nice to have). It seems we missed 1.2.15, so I updated doc comment about version availability, and I addressed your comments too. I guess I'll re-send it after the freeze.. I thought I've made it in, and we could say this was already sent before, but if you're not in a rush for this, It would be nice to push it after the release. No need to resend, though. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] storage: fs: Don't attempt directory creation if it already exists
On Mon, Apr 27, 2015 at 16:48:41 -0400, Cole Robinson wrote: > The current code attempts to handle this, but it only catches mkdir > failing with EEXIST. However if say trying to build /tmp for an > unprivileged qemu:///session, mkdir will fail with EPERM. > > Rather than catch any errors, just don't attempt mkdir if the directory > already exists. > --- > src/util/virfile.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index 87d121d..23a1655 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -2289,12 +2289,13 @@ virDirCreateNoFork(const char *path, > int ret = 0; > struct stat st; > > -if ((mkdir(path, mode) < 0) > -&& !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { > -ret = -errno; > -virReportSystemError(errno, _("failed to create directory '%s'"), > - path); > -goto error; > +if (!(flags & VIR_DIR_CREATE_ALLOW_EXIST) || !virFileExists(path)) { De Morgan's law allows to optimize this into a more readable form: if (!(virFileExists(path) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { It's one set of parentheses more but it's readable. > +if (mkdir(path, mode) < 0) { > +ret = -errno; > +virReportSystemError(errno, _("failed to create directory '%s'"), > + path); > +goto error; > +} ACK with the condition made human readable. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] storage: Fix a few issues with pool state tracking
On Mon, Apr 27, 2015 at 10:44:54AM -0400, Cole Robinson wrote: > First patch fixes a regression introduced with pool state patches > Last 3 patches help fix other issues with the state tracking > > Cole Robinson (4): > storage: Fix autostart dir for qemu:///session > storage: Don't leave stale state file if pool startup fails > storage: Break out storageDriverLoadPoolState > storage: If driver startup state syncing fails, delete statefile > > src/storage/storage_driver.c | 109 > ++- > 1 file changed, 67 insertions(+), 42 deletions(-) > ACK series Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] storage: fs: Fill in permissions on pool refresh
On Mon, Apr 27, 2015 at 16:48:40 -0400, Cole Robinson wrote: > This means pool XML actually reports accurate user/group/mode/label. > > This uses UpdateVolTargetInfoFD in a bit of a hackish way, but it works > --- > src/storage/storage_backend_fs.c | 58 > ++-- > 1 file changed, 44 insertions(+), 14 deletions(-) > > diff --git a/src/storage/storage_backend_fs.c > b/src/storage/storage_backend_fs.c > index 51d6bb3..804b7c3 100644 > --- a/src/storage/storage_backend_fs.c ... > +pool->def->target.perms.mode = target->perms->mode; > +pool->def->target.perms.uid = target->perms->uid; > +pool->def->target.perms.gid = target->perms->gid; > +VIR_FREE(pool->def->target.perms.label); > +if (VIR_STRDUP(pool->def->target.perms.label, target->perms->label) < 0) > +goto error; > > +ret = 0; > error: Since both success and error paths use this label now, it should be called 'cleanup'; > if (dir) > closedir(dir); > +VIR_FORCE_CLOSE(fd); > virStorageVolDefFree(vol); > -virStoragePoolObjClearVols(pool); > -return -1; > +virStorageSourceFree(target); > +if (ret < 0) > +virStoragePoolObjClearVols(pool); > +return ret; > } ACK with the label renamed. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-cim][PATCH] list_util.h: Drop inline modifiers
There's no need to mark a function as inline in the function declaration. In fact, it causes a compilation error: CC xmlgen.lo In file included from acl_parsing.h:29:0, from xmlgen.h:26, from capability_parsing.c:37: list_util.h:67:21: error: inline function ‘list_node_prev_node’ declared but never defined [-Werror] inline list_node_t *list_node_prev_node(list_node_t *node); ^ list_util.h:66:14: error: inline function ‘list_node_prev’ declared but never defined [-Werror] inline void*list_node_prev(list_node_t *node); ^ list_util.h:64:21: error: inline function ‘list_node_next_node’ declared but never defined [-Werror] inline list_node_t *list_node_next_node(list_node_t *node); ^ list_util.h:63:14: error: inline function ‘list_node_next’ declared but never defined [-Werror] inline void*list_node_next(list_node_t *node); ^ list_util.h:61:21: error: inline function ‘list_last_node’ declared but never defined [-Werror] inline list_node_t *list_last_node(list_t *list); ^ list_util.h:60:14: error: inline function ‘list_last’ declared but never defined [-Werror] inline void*list_last(list_t *list); ^ list_util.h:58:21: error: inline function ‘list_first_node’ declared but never defined [-Werror] inline list_node_t *list_first_node(list_t *list); ^ list_util.h:57:14: error: inline function ‘list_first’ declared but never defined [-Werror] inline void*list_first(list_t *list); ^ list_util.h:55:13: error: inline function ‘list_node_data_set’ declared but never defined [-Werror] inline void list_node_data_set(list_node_t *node, void *data); ^ list_util.h:54:14: error: inline function ‘list_node_data_get’ declared but never defined [-Werror] inline void *list_node_data_get(list_node_t *node); ^ list_util.h:52:21: error: inline function ‘list_count’ declared but never defined [-Werror] inline unsigned int list_count(list_t *list); ^ cc1: all warnings being treated as errors Makefile:499: recipe for target 'capability_parsing.lo' failed make[2]: *** [capability_parsing.lo] Error 1 make[2]: *** Waiting for unfinished jobs Signed-off-by: Michal Privoznik --- libxkutil/list_util.h | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/libxkutil/list_util.h b/libxkutil/list_util.h index 6510272..6582dfe 100644 --- a/libxkutil/list_util.h +++ b/libxkutil/list_util.h @@ -49,22 +49,22 @@ void list_remove_node(list_t *list, list_node_t *node); bool list_foreach(list_t *list, list_foreach_cb cb, void *user_data); -inline unsigned int list_count(list_t *list); +unsigned int list_count(list_t *list); -inline void *list_node_data_get(list_node_t *node); -inline void list_node_data_set(list_node_t *node, void *data); +void *list_node_data_get(list_node_t *node); +void list_node_data_set(list_node_t *node, void *data); -inline void*list_first(list_t *list); -inline list_node_t *list_first_node(list_t *list); +void*list_first(list_t *list); +list_node_t *list_first_node(list_t *list); -inline void*list_last(list_t *list); -inline list_node_t *list_last_node(list_t *list); +void*list_last(list_t *list); +list_node_t *list_last_node(list_t *list); -inline void*list_node_next(list_node_t *node); -inline list_node_t *list_node_next_node(list_node_t *node); +void*list_node_next(list_node_t *node); +list_node_t *list_node_next_node(list_node_t *node); -inline void*list_node_prev(list_node_t *node); -inline list_node_t *list_node_prev_node(list_node_t *node); +void*list_node_prev(list_node_t *node); +list_node_t *list_node_prev_node(list_node_t *node); #ifdef __cplusplus } /* extern "C" */ -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: fs: Ignore volumes that fail to open with EPERM
The summary says 'EPERM', but the code checks for EACCES. On Mon, Apr 27, 2015 at 11:57:53AM -0400, Cole Robinson wrote: > Trying to use qemu:///session to create a storage pool pointing at > /tmp will usually fail with something like: > > $ virsh pool-start tmp > error: Failed to start pool tmp > error: cannot open volume > '/tmp/systemd-private-c38cf0418d7a4734a66a8175996c384f-colord.service-kEyiTA': > Permission denied > > If any volume in an FS pool can't be opened by the daemon, the refresh > fails, and the pool can't be used. > > This causes pain for virt-install/virt-manager though. Imaging a user > downloads a disk image to /tmp. virt-manager wants to import /tmp as > a storage pool, so we can detect what disk format it is, and set the > XML correctly. However this case will likely fail as explained above. > > Change the logic here to skip volumes that fail to open. This could > conceivably cause user complaints along the lines of 'why doesn't > libvirt show $ROOT-OWNED-VOLUME-FOO', but figuring that currently > the pool won't even startup, I don't think there are any current > users that care about that case. > > https://bugzilla.redhat.com/show_bug.cgi?id=1103308 > --- > src/storage/storage_backend.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c > index e0311e1..186013c 100644 > --- a/src/storage/storage_backend.c > +++ b/src/storage/storage_backend.c > @@ -1445,6 +1445,10 @@ virStorageBackendVolOpen(const char *path, struct stat > *sb, > VIR_WARN("ignoring missing fifo '%s'", path); > return -2; > } > +if (errno == EACCES && noerror) { Looking at open(2) we are unlikely to hit EPERM, but maybe it would be better to look for both? > +VIR_WARN("ignore permission error for '%s'", path); s/ignore/ignoring/ to match the other warnings ACK whether you check EPERM or not, as long as you tweak the commit summary. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-perl PATCH] Add virDomainAddIOThread and virDomainDelIOThread support
On Tue, Apr 28, 2015 at 07:29:17AM -0400, John Ferlan wrote: > Add support for the new IOThread APIs > > Signed-off-by: John Ferlan > --- > Changes| 1 + > Virt.xs| 20 > lib/Sys/Virt/Domain.pm | 12 > 3 files changed, 33 insertions(+) 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] [libvirt-perl PATCH] Add virDomainAddIOThread and virDomainDelIOThread support
Add support for the new IOThread APIs Signed-off-by: John Ferlan --- Changes| 1 + Virt.xs| 20 lib/Sys/Virt/Domain.pm | 12 3 files changed, 33 insertions(+) diff --git a/Changes b/Changes index ead8f8e..58efa37 100644 --- a/Changes +++ b/Changes @@ -6,6 +6,7 @@ Revision history for perl module Sys::Virt event callback & constants. - Add JOB_DOWNTIME_NET constant - Add JOB_TIME_ELAPSED_NET constant + - Add virDomainAddIOThread and virDomainDelIOThread API bindings 1.2.14 2015-04-09 diff --git a/Virt.xs b/Virt.xs index debb8f4..29f20ec 100644 --- a/Virt.xs +++ b/Virt.xs @@ -5100,6 +5100,26 @@ PREINIT: _croak_error(); +void +add_iothread(dom, iothread_id, flags=0) + virDomainPtr dom; + unsigned int iothread_id; + unsigned int flags; + PPCODE: + if (virDomainAddIOThread(dom, iothread_id, flags) < 0) + _croak_error(); + + +void +del_iothread(dom, iothread_id, flags=0) + virDomainPtr dom; + unsigned int iothread_id; + unsigned int flags; + PPCODE: + if (virDomainDelIOThread(dom, iothread_id, flags) < 0) + _croak_error(); + + int num_of_snapshots(dom, flags=0) virDomainPtr dom; diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index f12abd1..2fda74d 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1237,6 +1237,18 @@ Pin the IOThread given by index C<$iothread> to physical CPUs given by C<$mask>. The C<$mask> is a string representing a bitmask against physical CPUs, 8 cpus per character. +=item $dom->add_iothread($iothread, $flags=0) + +Add a new IOThread by the C<$iothread> value to the guest domain. +The C<$flags> parameter accepts one or more the CONFIG OPTION constants +documented later, and defaults to 0 if omitted. + +=item $dom->del_iothread($iothread, $flags=0) + +Delete an existing IOThread by the C<$iothread> value from the guest domain. +The C<$flags> parameter accepts one or more the CONFIG OPTION constants +documented later, and defaults to 0 if omitted. + =item my @stats = $dom->get_cpu_stats($startCpu, $numCpus, $flags=0) Requests the guests host physical CPU usage statistics, starting -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/3] IOThread algorithm followups
On 04/28/2015 06:40 AM, John Ferlan wrote: > v1 here: > http://www.redhat.com/archives/libvir-list/2015-April/msg01382.html > > v2 changes: > > Patch 1 - Add a !STRPREFIX(tmp, "iothread") of the returned iothreads > to ignore anything that doesn't start with "iothread". > > Patch 2 - already ACKed, just difficult to extract. > > Patch 3 - NEW: remove qemuMonitorIOThreadInfoFree as requested from > review of patch 1. > > John Ferlan (3): > qemu: Remove need for qemuDomainParseIOThreadAlias > qemu: qemuProcessDetectIOThreadPIDs invert checks > qemu: Remove need for qemuMonitorIOThreadInfoFree > > src/qemu/qemu_command.c | 17 - > src/qemu/qemu_command.h | 3 --- > src/qemu/qemu_driver.c | 15 +-- > src/qemu/qemu_monitor.c | 10 -- > src/qemu/qemu_monitor.h | 4 +--- > src/qemu/qemu_monitor_json.c | 22 +++--- > src/qemu/qemu_process.c | 25 +++-- > tests/qemumonitorjsontest.c | 14 +++--- > 8 files changed, 39 insertions(+), 71 deletions(-) > Thanks for the reviews - now pushed. John FYI: This cover letter touched on why patch 2 was a repeat... It was just easier to not extract it - perhaps the git merge would have worked, perhaps it would not have. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix building virnetserverclientmock with MinGW
Signed-off-by: Pavel Fedin Signed-off-by: Martin Kletzander --- Notes: This was a part of commit 9dc57ce2, but separable; plus the library should be in LIBADD and not LDFLAGS. Depends on my previous cleanup [2] [1] https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9dc57ce2 [2] https://www.redhat.com/archives/libvir-list/2015-April/msg01336.html v2: Use LIBADD instead of LDADDS (which doesn't even exist) tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index dc7daaa..8e2dbec 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -931,6 +931,7 @@ virnetserverclientmock_la_SOURCES = \ virnetserverclientmock.c virnetserverclientmock_la_CFLAGS = $(AM_CFLAGS) virnetserverclientmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virnetserverclientmock_la_LIBADD = $(GNULIB_LIBS) if WITH_GNUTLS virnettlscontexttest_SOURCES = \ -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add missing linker flags for MinGW build
On Tue, Apr 28, 2015 at 11:40:02AM +0300, Pavel Fedin wrote: Hello! I have checked. For some reason, $(virnetserverclientmock_la_LDADDS) does not take part anywhere. Here are generated rules: It has to be virnetserverclientmock_la_LIBADD instead. I have tested, works fine. I just wrote that and was waiting with an answer until I can check whether that helps. Yes it needs to be LIBADD, I'll send a v2. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] qemu: Remove need for qemuMonitorIOThreadInfoFree
On Tue, Apr 28, 2015 at 06:40:32 -0400, John Ferlan wrote: > Replace with just VIR_FREE. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_driver.c | 6 +++--- > src/qemu/qemu_monitor.c | 9 - > src/qemu/qemu_monitor.h | 2 -- > src/qemu/qemu_monitor_json.c | 2 +- > src/qemu/qemu_process.c | 2 +- > tests/qemumonitorjsontest.c | 2 +- > 6 files changed, 6 insertions(+), 17 deletions(-) > ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] qemu: Remove need for qemuDomainParseIOThreadAlias
On Tue, Apr 28, 2015 at 06:40:30 -0400, John Ferlan wrote: > Rather than have a separate routine to parse the alias of an iothread > returned from qemu in order to get the iothread_id value, parse the alias > when returning and just return the iothread_id in qemuMonitorIOThreadInfoPtr > > This set of patches removes the function, changes the "char *name" to > "unsigned int" and handles all the fallout. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_command.c | 17 - > src/qemu/qemu_command.h | 3 --- > src/qemu/qemu_driver.c | 9 ++--- > src/qemu/qemu_monitor.c | 1 - > src/qemu/qemu_monitor.h | 2 +- > src/qemu/qemu_monitor_json.c | 20 ++-- > src/qemu/qemu_process.c | 11 --- > tests/qemumonitorjsontest.c | 12 ++-- > 8 files changed, 27 insertions(+), 48 deletions(-) > ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] qemu: qemuProcessDetectIOThreadPIDs invert checks
On Tue, Apr 28, 2015 at 06:40:31 -0400, John Ferlan wrote: > If we received zero iothreads from the monitor, but were perhaps > expecting to receive something, then the code was skipping the check > to ensure what's in the monitor matches our expectations. So invert > the checks to check that what we get back matches expectations and > then check there are zero iothreads returned. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_process.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > This was already acked and separate. You should have pushed it already. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] FYI: Hacking on Sys::Virt libvirt Perl binding
Historically I have taken care of everything related to the Sys::Virt Perl binding for libvirt. Recently a couple of adventurous people have started sending patches too. Perl may have a reputation for being scary, but at this point extending the Sys::Virt binding when new APIs or constants are added is actually pretty straightforward. The majority of new libvirt APIs calls look very similar to one or more existing APIs, so you can usually get away with just doing a copy+paste job from some existing Perl API binding. To assist people in understanding how todo this, I've updated the HACKING file with some guidance. I'm including it inline here for reference. I'm not expecting everyone to submit Perl binding code for APIs they add. If people do submit binding code for new APIs though, it will be more than welcome - if not I'll continue to watch for changes and update things myself when required. Hacking on libvirt perl === The libvirt Perl release versions are tied directly to the libvirt C library release versions. ie Sys::Virt 1.2.10 will require libvirt version 1.2.10 or newer in order to build. We do not aim to support conditional compilation against versions of libvirt that are older than the version of Sys::Virt. General changes for new APIs Additions to the libvirt C API will require changes to a minimum of two parts of the Sys::Virt codebase. - Virt.xs - this provides the C glue code to access the libvirt C library APIs and constants from the Perl interpretor. As a general rule, every new function and header file constant/enum requires an addition to this file. The exceptions are functions that are only provided for the benefit of language bindings and not intended for use by application code. For example the reference counting APIs don't need exposing to Perl applications - lib/ - this directory contains the pure Perl part of the binding. There are separate files for each core libvirt object type - lib/Sys/Virt.pm - mapping for virConnectPtr - lib/Sys/Virt/Domain.pm - mapping for virDomainPtr - lib/Sys/Virt/Error.pm - mapping for virErrorPtr - lib/Sys/Virt/Event.pm - mapping for virEventPtr - lib/Sys/Virt/Interface.pm - mapping for virInterfacePtr - lib/Sys/Virt/Network.pm - mapping for virNetworkPtr - lib/Sys/Virt/NodeDevice.pm - mapping for virNodeDevicePtr - lib/Sys/Virt/NWFilter.pm - mapping for virNWFilterPtr - lib/Sys/Virt/Secret.pm - mapping for virSecretPtr - lib/Sys/Virt/StoragePool.pm - mapping for virStoragePoolPtr - lib/Sys/Virt/StorageVol.pm - mapping for virStorageVolPtr - lib/Sys/Virt/Stream.pm - mapping for virStreamPtr There is rarely a need to write Perl code in the .pm modules, as the mapping in the Virt.xs file is usually sufficient. As such the primary purpose of the .pm modules is to hold the POD inline documentation. Every function and constants is required to have full API documentation provided There are a number of unit tests available in the t/ directory which assist in creation of new APIs. - t/010-pod-coverage.t - ensures that every Perl method and constant has POD documentation present - t/030-api-coverage.t - ensures that every C library method/constant in the libvirt-api.xml file has corresponding code in the Virt.xs. Certain functions can be blacklisted in t/030-api-coverage.t as not needed mapping to Perl. This only runs if TEST_MAINTAINER=1 is set. - t/*.t - the other files mostly do functional testing against the test:///default API - if the new function has support in the test driver, then suitable additions should be made If use of the API is not obvious, it is often worth providing a small example program in the examples/ directory. These examples are also useful when adding APIs to ensure that they are operating correctly, if it wasn't possible to unit test them with test:///default. Every addition / change to the API must be documented in the Changes file. New API addition workflow - When the libvirt C library is changed, the following workflow is an effective way to update the Perl binding. - Build the libvirt C library # cd $HOME/src/libvirt # ./autogen.sh --system # make - Configure & build the Sys::Virt module to build against the just built libvirt library # cd $HOME/src/libvirt-perl # ../libvirt/run perl Makefile.PL # ../libvirt/run make - Run the test suite to identify which new functions/constants need handling # ../libvirt/run make test TEST_MAINTAINER=1 - For each missing item reported in the test suite... - Edit Virt.xs to add the C binding - Edit lib/*.pm to add the POD documentation (and occassionally Perl glue code) - Edit Changes to document the addition - Run the test suite (without maintainer mode) to verify POD docs # ../libvirt/run make test - Optiona
[libvirt] [PATCH v2 3/3] qemu: Remove need for qemuMonitorIOThreadInfoFree
Replace with just VIR_FREE. Signed-off-by: John Ferlan --- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_monitor.c | 9 - src/qemu/qemu_monitor.h | 2 -- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d0147e9..80463f2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5950,7 +5950,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, } if (iothreads) { for (i = 0; i < niothreads; i++) -qemuMonitorIOThreadInfoFree(iothreads[i]); +VIR_FREE(iothreads[i]); VIR_FREE(iothreads); } @@ -6330,7 +6330,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, cleanup: if (new_iothreads) { for (idx = 0; idx < new_niothreads; idx++) -qemuMonitorIOThreadInfoFree(new_iothreads[idx]); +VIR_FREE(new_iothreads[idx]); VIR_FREE(new_iothreads); } VIR_FREE(mem_mask); @@ -6416,7 +6416,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, cleanup: if (new_iothreads) { for (idx = 0; idx < new_niothreads; idx++) -qemuMonitorIOThreadInfoFree(new_iothreads[idx]); +VIR_FREE(new_iothreads[idx]); VIR_FREE(new_iothreads); } virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 48bfeb0..6be3ca2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3813,15 +3813,6 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon, } -void -qemuMonitorIOThreadInfoFree(qemuMonitorIOThreadInfoPtr iothread) -{ -if (!iothread) -return; -VIR_FREE(iothread); -} - - /** * qemuMonitorGetMemoryDeviceInfo: * @mon: pointer to the monitor diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index bce8031..a07e43b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -883,8 +883,6 @@ struct _qemuMonitorIOThreadInfo { int qemuMonitorGetIOThreads(qemuMonitorPtr mon, qemuMonitorIOThreadInfoPtr **iothreads); -void qemuMonitorIOThreadInfoFree(qemuMonitorIOThreadInfoPtr iothread); - typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo; typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c02ef47..e281140 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6518,7 +6518,7 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, cleanup: if (ret < 0 && infolist) { for (i = 0; i < n; i++) -qemuMonitorIOThreadInfoFree(infolist[i]); +VIR_FREE(infolist[i]); VIR_FREE(infolist); } virJSONValueFree(cmd); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d8a747c..605b3c6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2265,7 +2265,7 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, cleanup: if (iothreads) { for (i = 0; i < niothreads; i++) -qemuMonitorIOThreadInfoFree(iothreads[i]); +VIR_FREE(iothreads[i]); VIR_FREE(iothreads); } return ret; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 39eeaa7..c6379b6 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2295,7 +2295,7 @@ testQemuMonitorJSONGetIOThreads(const void *data) cleanup: qemuMonitorTestFree(test); for (i = 0; i < ninfo; i++) -qemuMonitorIOThreadInfoFree(info[i]); +VIR_FREE(info[i]); VIR_FREE(info); return ret; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] IOThread algorithm followups
v1 here: http://www.redhat.com/archives/libvir-list/2015-April/msg01382.html v2 changes: Patch 1 - Add a !STRPREFIX(tmp, "iothread") of the returned iothreads to ignore anything that doesn't start with "iothread". Patch 2 - already ACKed, just difficult to extract. Patch 3 - NEW: remove qemuMonitorIOThreadInfoFree as requested from review of patch 1. John Ferlan (3): qemu: Remove need for qemuDomainParseIOThreadAlias qemu: qemuProcessDetectIOThreadPIDs invert checks qemu: Remove need for qemuMonitorIOThreadInfoFree src/qemu/qemu_command.c | 17 - src/qemu/qemu_command.h | 3 --- src/qemu/qemu_driver.c | 15 +-- src/qemu/qemu_monitor.c | 10 -- src/qemu/qemu_monitor.h | 4 +--- src/qemu/qemu_monitor_json.c | 22 +++--- src/qemu/qemu_process.c | 25 +++-- tests/qemumonitorjsontest.c | 14 +++--- 8 files changed, 39 insertions(+), 71 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] qemu: qemuProcessDetectIOThreadPIDs invert checks
If we received zero iothreads from the monitor, but were perhaps expecting to receive something, then the code was skipping the check to ensure what's in the monitor matches our expectations. So invert the checks to check that what we get back matches expectations and then check there are zero iothreads returned. Signed-off-by: John Ferlan --- src/qemu/qemu_process.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f06ec56..d8a747c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2233,12 +2233,6 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, if (niothreads < 0) goto cleanup; -/* Nothing to do */ -if (niothreads == 0) { -ret = 0; -goto cleanup; -} - if (niothreads != vm->def->iothreads) { virReportError(VIR_ERR_INTERNAL_ERROR, _("got wrong number of IOThread pids from QEMU monitor. " @@ -2247,6 +2241,12 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, goto cleanup; } +/* Nothing to do */ +if (niothreads == 0) { +ret = 0; +goto cleanup; +} + for (i = 0; i < niothreads; i++) { virDomainIOThreadIDDefPtr iothrid; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] qemu: Remove need for qemuDomainParseIOThreadAlias
Rather than have a separate routine to parse the alias of an iothread returned from qemu in order to get the iothread_id value, parse the alias when returning and just return the iothread_id in qemuMonitorIOThreadInfoPtr This set of patches removes the function, changes the "char *name" to "unsigned int" and handles all the fallout. Signed-off-by: John Ferlan --- src/qemu/qemu_command.c | 17 - src/qemu/qemu_command.h | 3 --- src/qemu/qemu_driver.c | 9 ++--- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 20 ++-- src/qemu/qemu_process.c | 11 --- tests/qemumonitorjsontest.c | 12 ++-- 8 files changed, 27 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 247954f..ba15dc9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -677,23 +677,6 @@ qemuOpenVhostNet(virDomainDefPtr def, return -1; } -int -qemuDomainParseIOThreadAlias(char *alias, - unsigned int *iothread_id) -{ -unsigned int idval; - -if (virStrToLong_ui(alias + strlen("iothread"), -NULL, 10, &idval) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to find iothread id for '%s'"), - alias); -return -1; -} - -*iothread_id = idval; -return 0; -} int qemuNetworkPrepareDevices(virDomainDefPtr def) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 538ccdf..a29db41 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -238,9 +238,6 @@ int qemuOpenVhostNet(virDomainDefPtr def, int *vhostfd, size_t *vhostfdSize); -int qemuDomainParseIOThreadAlias(char *alias, - unsigned int *iothread_id); - int qemuNetworkPrepareDevices(virDomainDefPtr def); /* diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 74dcb0a..d0147e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5918,16 +5918,11 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, goto endjob; for (i = 0; i < niothreads; i++) { -unsigned int iothread_id; virBitmapPtr map = NULL; -if (qemuDomainParseIOThreadAlias(iothreads[i]->name, - &iothread_id) < 0) -goto endjob; - if (VIR_ALLOC(info_ret[i]) < 0) goto endjob; -info_ret[i]->iothread_id = iothread_id; +info_ret[i]->iothread_id = iothreads[i]->iothread_id; if (virProcessGetAffinity(iothreads[i]->thread_id, &map, hostcpus) < 0) goto endjob; @@ -6292,7 +6287,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver, * in the QEMU IOThread list, so we can add it to our iothreadids list */ for (idx = 0; idx < new_niothreads; idx++) { -if (STREQ(new_iothreads[idx]->name, alias)) +if (new_iothreads[idx]->iothread_id == iothread_id) break; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1e7d2ef..48bfeb0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3818,7 +3818,6 @@ qemuMonitorIOThreadInfoFree(qemuMonitorIOThreadInfoPtr iothread) { if (!iothread) return; -VIR_FREE(iothread->name); VIR_FREE(iothread); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cd4cc66..bce8031 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -877,7 +877,7 @@ typedef struct _qemuMonitorIOThreadInfo qemuMonitorIOThreadInfo; typedef qemuMonitorIOThreadInfo *qemuMonitorIOThreadInfoPtr; struct _qemuMonitorIOThreadInfo { -char *name; +unsigned int iothread_id; int thread_id; }; int qemuMonitorGetIOThreads(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3af319c..c02ef47 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6481,20 +6481,28 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, const char *tmp; qemuMonitorIOThreadInfoPtr info; -if (VIR_ALLOC(info) < 0) -goto cleanup; - -infolist[i] = info; - if (!(tmp = virJSONValueObjectGetString(child, "id"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-iothreads reply data was missing 'id'")); goto cleanup; } -if (VIR_STRDUP(info->name, tmp) < 0) +if (!STRPREFIX(tmp, "iothread")) +continue; + +if (VIR_ALLOC(info) < 0) goto cleanup; +infolist[i] = info; + +if (virStrToLong_ui(tmp + strlen("iothread"), +NULL, 10, &info->iothread_id) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, +
Re: [libvirt] [perl][PATCH 4/4] Add JOB_TIME_ELAPSED_NET constant
On Tue, Apr 28, 2015 at 11:54:07AM +0200, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > Changes| 1 + > Virt.xs| 1 + > lib/Sys/Virt/Domain.pm | 6 ++ > 3 files changed, 8 insertions(+) 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
Re: [libvirt] [perl][PATCH 3/4] Add JOB_DOWNTIME_NET constant
On Tue, Apr 28, 2015 at 11:54:06AM +0200, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > Changes| 1 + > Virt.xs| 1 + > lib/Sys/Virt/Domain.pm | 6 ++ > 3 files changed, 8 insertions(+) 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
Re: [libvirt] [perl][PATCH 2/4] Add support for VIR_DOMAIN_EVENT_ID_DEVICE_ADDED
On Tue, Apr 28, 2015 at 11:54:05AM +0200, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > Changes| 3 ++- > Virt.xs| 42 ++ > lib/Sys/Virt/Domain.pm | 4 > t/030-api-coverage.t | 1 + > 4 files changed, 49 insertions(+), 1 deletion(-) 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
Re: [libvirt] [perl][PATCH 1/4] domain_event_register_any: Align the code
On Tue, Apr 28, 2015 at 11:54:04AM +0200, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > Virt.xs | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Virt.xs b/Virt.xs > index d01cf05..4b3ba9a 100644 > --- a/Virt.xs > +++ b/Virt.xs > @@ -2946,8 +2946,8 @@ PREINIT: >callback = > VIR_DOMAIN_EVENT_CALLBACK(_domain_event_device_removed_callback); >break; >case VIR_DOMAIN_EVENT_ID_TUNABLE: > - callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_tunable_callback); > - break; > + callback = > VIR_DOMAIN_EVENT_CALLBACK(_domain_event_tunable_callback); > + break; >case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE: >callback = > VIR_DOMAIN_EVENT_CALLBACK(_domain_event_agent_lifecycle_callback); >break; 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] [perl][PATCH 0/4] Fix coverage
In order to match the newish things in libvirt. Michal Privoznik (4): domain_event_register_any: Align the code Add support for VIR_DOMAIN_EVENT_ID_DEVICE_ADDED Add JOB_DOWNTIME_NET constant Add JOB_TIME_ELAPSED_NET constant Changes| 5 - Virt.xs| 48 ++-- lib/Sys/Virt/Domain.pm | 16 t/030-api-coverage.t | 1 + 4 files changed, 67 insertions(+), 3 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl][PATCH 3/4] Add JOB_DOWNTIME_NET constant
Signed-off-by: Michal Privoznik --- Changes| 1 + Virt.xs| 1 + lib/Sys/Virt/Domain.pm | 6 ++ 3 files changed, 8 insertions(+) diff --git a/Changes b/Changes index a299dc7..7a7ba4c 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,7 @@ Revision history for perl module Sys::Virt - Add support for VIR_DOMAIN_EVENT_ID_DEVICE_ADDED event callback & constants. + - Add JOB_DOWNTIME_NET constant 1.2.14 2015-04-09 diff --git a/Virt.xs b/Virt.xs index 40a5ee8..04aaf90 100644 --- a/Virt.xs +++ b/Virt.xs @@ -7560,6 +7560,7 @@ BOOT: REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_DISK_TOTAL, JOB_DISK_TOTAL); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_DISK_BPS, JOB_DISK_BPS); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_DOWNTIME, JOB_DOWNTIME); + REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_DOWNTIME_NET, JOB_DOWNTIME_NET); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_CONSTANT, JOB_MEMORY_CONSTANT); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_NORMAL, JOB_MEMORY_NORMAL); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_NORMAL_BYTES, JOB_MEMORY_NORMAL_BYTES); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 2bf3f8b..3c04169 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1423,6 +1423,12 @@ non-compressed page. The number of milliseconds of downtime expected during migration switchover. +=item Sys::Virt::Domain::JOB_DOWNTIME_NET + +Real measured downtime (ms) NOT including the time required to +transfer control flow from the source host to the destination +host. + =item Sys::Virt::Domain::JOB_SETUP_TIME The number of milliseconds of time doing setup of the job -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl][PATCH 2/4] Add support for VIR_DOMAIN_EVENT_ID_DEVICE_ADDED
Signed-off-by: Michal Privoznik --- Changes| 3 ++- Virt.xs| 42 ++ lib/Sys/Virt/Domain.pm | 4 t/030-api-coverage.t | 1 + 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Changes b/Changes index 35ec792..a299dc7 100644 --- a/Changes +++ b/Changes @@ -2,7 +2,8 @@ Revision history for perl module Sys::Virt 1.2.15 2015-00-00 - - ... + - Add support for VIR_DOMAIN_EVENT_ID_DEVICE_ADDED + event callback & constants. 1.2.14 2015-04-09 diff --git a/Virt.xs b/Virt.xs index 4b3ba9a..40a5ee8 100644 --- a/Virt.xs +++ b/Virt.xs @@ -858,6 +858,44 @@ _domain_event_balloonchange_callback(virConnectPtr con, static int +_domain_event_device_added_callback(virConnectPtr con, +virDomainPtr dom, +const char *devAlias, +void *opaque) +{ +AV *data = opaque; +SV **self; +SV **cb; +SV *domref; +dSP; + +self = av_fetch(data, 0, 0); +cb = av_fetch(data, 1, 0); + +SvREFCNT_inc(*self); + +ENTER; +SAVETMPS; + +PUSHMARK(SP); +XPUSHs(*self); +domref = sv_newmortal(); +sv_setref_pv(domref, "Sys::Virt::Domain", (void*)dom); +virDomainRef(dom); +XPUSHs(domref); +XPUSHs(sv_2mortal(newSVpv(devAlias, 0))); +PUTBACK; + +call_sv(*cb, G_DISCARD); + +FREETMPS; +LEAVE; + +return 0; +} + + +static int _domain_event_device_removed_callback(virConnectPtr con, virDomainPtr dom, const char *devAlias, @@ -2942,6 +2980,9 @@ PREINIT: case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE: callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_balloonchange_callback); break; + case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED: + callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_device_added_callback); + break; case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED: callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_device_removed_callback); break; @@ -7563,6 +7604,7 @@ BOOT: REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_PMWAKEUP, EVENT_ID_PMWAKEUP); REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_TRAY_CHANGE, EVENT_ID_TRAY_CHANGE); REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE, EVENT_ID_BALLOON_CHANGE); + REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_DEVICE_ADDED, EVENT_ID_DEVICE_ADDED); REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, EVENT_ID_DEVICE_REMOVED); REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_TUNABLE, EVENT_ID_TUNABLE); REGISTER_CONSTANT(VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE, EVENT_ID_AGENT_LIFECYCLE); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 7e2d0c0..2bf3f8b 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -2791,6 +2791,10 @@ Power management initiated wakeup Balloon target changes +=item Sys::Virt::Domain::EVENT_ID_DEVICE_ADDED + +Asynchronous guest device addition + =item Sys::Virt::Domain::EVENT_ID_DEVICE_REMOVED Asynchronous guest device removal diff --git a/t/030-api-coverage.t b/t/030-api-coverage.t index 592f1d3..9ce6155 100644 --- a/t/030-api-coverage.t +++ b/t/030-api-coverage.t @@ -87,6 +87,7 @@ virConnectDomainEventPMSuspendDiskCallback virConnectDomainEventPMWakeupCallback virConnectDomainEventTrayChangeCallback virConnectDomainEventBalloonChangeCallback +virConnectDomainEventDeviceAddedCallback virConnectDomainEventDeviceRemovedCallback virConnectDomainEventTunableCallback virConnectDomainEventAgentLifecycleCallback -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl][PATCH 4/4] Add JOB_TIME_ELAPSED_NET constant
Signed-off-by: Michal Privoznik --- Changes| 1 + Virt.xs| 1 + lib/Sys/Virt/Domain.pm | 6 ++ 3 files changed, 8 insertions(+) diff --git a/Changes b/Changes index 7a7ba4c..ead8f8e 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,7 @@ Revision history for perl module Sys::Virt - Add support for VIR_DOMAIN_EVENT_ID_DEVICE_ADDED event callback & constants. - Add JOB_DOWNTIME_NET constant + - Add JOB_TIME_ELAPSED_NET constant 1.2.14 2015-04-09 diff --git a/Virt.xs b/Virt.xs index 04aaf90..debb8f4 100644 --- a/Virt.xs +++ b/Virt.xs @@ -7570,6 +7570,7 @@ BOOT: REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_BPS, JOB_MEMORY_BPS); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_SETUP_TIME, JOB_SETUP_TIME); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_TIME_ELAPSED, JOB_TIME_ELAPSED); + REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_TIME_ELAPSED_NET, JOB_TIME_ELAPSED_NET); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_TIME_REMAINING, JOB_TIME_REMAINING); REGISTER_CONSTANT(VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN, BLOCK_JOB_TYPE_UNKNOWN); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 3c04169..f12abd1 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1334,6 +1334,12 @@ Return the stats of the most recently completed job. The elapsed time in milliseconds +=item Sys::Virt::Domain::JOB_TIME_ELAPSED_NET + +Time in miliseconds since the beginning of the migration job NOT +including the time required to transfer control flow from the +source host to the destination host. + =item Sys::Virt::Domain::JOB_TIME_REMAINING The expected remaining time in milliseconds. Only set if the -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl][PATCH 1/4] domain_event_register_any: Align the code
Signed-off-by: Michal Privoznik --- Virt.xs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Virt.xs b/Virt.xs index d01cf05..4b3ba9a 100644 --- a/Virt.xs +++ b/Virt.xs @@ -2946,8 +2946,8 @@ PREINIT: callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_device_removed_callback); break; case VIR_DOMAIN_EVENT_ID_TUNABLE: - callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_tunable_callback); - break; + callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_tunable_callback); + break; case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE: callback = VIR_DOMAIN_EVENT_CALLBACK(_domain_event_agent_lifecycle_callback); break; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/6] Add vmport feature
Hi Martin On Mon, Apr 27, 2015 at 4:20 PM, Martin Kletzander wrote: > ACK series with two things to consider mentioned in-line (not hard > requirements, but it would be nice to have). > It seems we missed 1.2.15, so I updated doc comment about version availability, and I addressed your comments too. I guess I'll re-send it after the freeze.. -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
On 28.04.2015 11:06, Pavel Boldin wrote: > Well, actually that seems to be quite a different bug in there. > > I will start a new thread. > > In short: migration seems to be broken by commit > 1a92c719101e5bfa6fe2b78006ad04c7f075ea28. This is because introduced job > _MODIFY waits while MIGRATION_OUT is finished to change `mirrorState' > variable. This deadlocks the libvirt. Yep, this is known bug. I've told Peter already like two weeks ago. He promised to fix it. It would be nice if we can get the fix into the release. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
Well, actually that seems to be quite a different bug in there. I will start a new thread. In short: migration seems to be broken by commit 1a92c719101e5bfa6fe2b78006ad04c7f075ea28. This is because introduced job _MODIFY waits while MIGRATION_OUT is finished to change `mirrorState' variable. This deadlocks the libvirt. Pavel On Tue, Apr 28, 2015 at 6:29 AM, Pavel Boldin wrote: > Dear Libvirt Developers, > > There seems to be a bug or at least a bad behavior in > `src/qemu/qemu_monitor.c' lines 683-689 function `qemuMonitorIO': > > if (qemuMonitorIOWrite(mon) < 0) { > error = true; > if (errno == ECONNRESET) > hangup = true; > } > events &= ~VIR_EVENT_HANDLE_WRITABLE; > > The `qemuMonitorIOWrite' is returning 0 in case 'write' returns EAGAIN > thus 'events' is always cleared of `VIR_EVENT_HANDLE_WRITABLE' even in case > no message have been sent indeed. > > The question is: who is responsible for handling this? It seems like > 'errno' is getting overwritten inside all of qemuMonitorJSON* functions so > the caller can't rely on it and it needs to be fixed inside the > `qemuMonitorIO'. > > Pavel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/2] conf: Resolve some Coverity errors
On Mon, Apr 27, 2015 at 15:08:42 -0400, John Ferlan wrote: > Resolve some Coverity errors with IOThread changes > > Signed-off-by: John Ferlan > --- > > I ran my Coverity checker too, but I looked at the wrong results > tab when I went to check the results... I looked at a previous run > which was clean, not the most recent run which wasn't. Doing > multiple compiles in separate tabs and just couldn't keep it all > straight ( - old age I guess) > > src/conf/domain_conf.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 0b18720..fc48ed5 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -13253,6 +13253,7 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node, > > error: > virDomainIOThreadIDDefFree(iothrid); > +iothrid = NULL; > goto cleanup; > } > > @@ -13342,7 +13343,7 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node, > { > int ret = -1; > virDomainIOThreadIDDefPtr iothrid; > -virBitmapPtr cpumask; > +virBitmapPtr cpumask = NULL; > xmlNodePtr oldnode = ctxt->node; > unsigned int iothreadid; > char *tmp = NULL; I've already ACKed Roman's patch that fixes this. > @@ -13372,6 +13373,7 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node, > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > _("Cannot find 'iothread' : %u"), > iothreadid); > +goto cleanup; > } > > if (!(tmp = virXMLPropString(node, "cpuset"))) { ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: qemuProcessDetectIOThreadPIDs invert checks
On Mon, Apr 27, 2015 at 14:51:05 -0400, John Ferlan wrote: > If we received zero iothreads from the monitor, but were perhaps > expecting to receive something, then the code was skipping the check > to ensure what's in the monitor matches our expectations. So invert > the checks to check that what we get back matches expectations and > then check there are zero iothreads returned Missing full stop at the end of the sentence. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_process.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Remove need for qemuDomainParseIOThreadAlias
On Mon, Apr 27, 2015 at 14:51:04 -0400, John Ferlan wrote: > Rather than have a separate routine to parse the alias of an iothread > returned from qemu in order to get the iothread_id value, parse the alias > when returning and just return the iothread_id in qemuMonitorIOThreadInfoPtr > > This set of patches removes the function, changes the "char *name" to > "unsigned int" and handles all the fallout. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_command.c | 17 - > src/qemu/qemu_command.h | 3 --- > src/qemu/qemu_driver.c | 9 ++--- > src/qemu/qemu_monitor.c | 1 - > src/qemu/qemu_monitor.h | 2 +- > src/qemu/qemu_monitor_json.c | 7 ++- > src/qemu/qemu_process.c | 11 --- > tests/qemumonitorjsontest.c | 12 ++-- > 8 files changed, 19 insertions(+), 43 deletions(-) > ... > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 1e7d2ef..48bfeb0 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -3818,7 +3818,6 @@ qemuMonitorIOThreadInfoFree(qemuMonitorIOThreadInfoPtr > iothread) > { > if (!iothread) > return; > -VIR_FREE(iothread->name); > VIR_FREE(iothread); Looks like this function can be killed and replaced with VIR_FREE(). > } > ... > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 3af319c..76687ff 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -6492,8 +6492,13 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, > goto cleanup; > } > > -if (VIR_STRDUP(info->name, tmp) < 0) > +if (virStrToLong_ui(tmp + strlen("iothread"), You shouldn't assume that the returned thread will begin with iothread. The code should make sure that the STRPREFIX is "iothread" before moving the pointer. If the alias will be shorter it will crash. > +NULL, 10, &info->iothread_id) < 0) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("failed to find iothread id for '%s'"), > + tmp); > goto cleanup; > +} > > if (virJSONValueObjectGetNumberInt(child, "thread-id", > &info->thread_id) < 0) { Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add missing linker flags for MinGW build
Hello! > I have checked. For some reason, $(virnetserverclientmock_la_LDADDS) > does not take part anywhere. Here are generated rules: It has to be virnetserverclientmock_la_LIBADD instead. I have tested, works fine. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] storage: fs: Don't overwrite virDirCreate error
On Mon, Apr 27, 2015 at 16:48:39 -0400, Cole Robinson wrote: > virDirCreate will give us fine grained details about what actually failed. > --- > src/storage/storage_backend_fs.c | 4 > 1 file changed, 4 deletions(-) ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: explicitly initialize 'cpumask' variable
Peter Krempa wrote: > On Tue, Apr 28, 2015 at 08:30:30 +0400, Roman Bogorodskiy wrote: > > Build with clang fails with: > > > > CC conf/libvirt_conf_la-domain_conf.lo > > conf/domain_conf.c:13377:9: error: variable 'cpumask' is used > > uninitialized whenever 'if' condition is true > > [-Werror,-Wsometimes-uninitialized] > > if (!(tmp = virXMLPropString(node, "cpuset"))) { > > ^ > > > > and many other similar errors regarding the 'cpuset' variable. > > > > Fix by explicitly initializing it with NULL. > > --- > > ACK, qualifies for both build-breaker and trivial rule. Pushed, thanks! Roman Bogorodskiy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: explicitly initialize 'cpumask' variable
On Tue, Apr 28, 2015 at 08:30:30 +0400, Roman Bogorodskiy wrote: > Build with clang fails with: > > CC conf/libvirt_conf_la-domain_conf.lo > conf/domain_conf.c:13377:9: error: variable 'cpumask' is used > uninitialized whenever 'if' condition is true > [-Werror,-Wsometimes-uninitialized] > if (!(tmp = virXMLPropString(node, "cpuset"))) { > ^ > > and many other similar errors regarding the 'cpuset' variable. > > Fix by explicitly initializing it with NULL. > --- ACK, qualifies for both build-breaker and trivial rule. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add missing linker flags for MinGW build
Hello! > If it works, feel free to close the BZ or reply if it doesn't. I have checked. For some reason, $(virnetserverclientmock_la_LDADDS) does not take part anywhere. Here are generated rules: --- cut --- virnetserverclientmock_la_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC \ $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=link $(CCLD) \ $(virnetserverclientmock_la_CFLAGS) $(CFLAGS) \ $(virnetserverclientmock_la_LDFLAGS) $(LDFLAGS) -o $@ --- cut --- virnetserverclientmock.la: $(virnetserverclientmock_la_OBJECTS) $(virnetserverclientmock_la_DEPENDENCIES) $(EXTRA_virnetserverclientmock_la_DEPENDENCIES) $(AM_V_CCLD)$(virnetserverclientmock_la_LINK) $(am_virnetserverclientmock_la_rpath) $(virnetserverclientmock_la_OBJECTS) $(virnetserverclientmock_la_LIBADD) $(LIBS) --- cut --- I have looked through Makefile.am, xxx_LDADD seems to be used only for executables, and not for shared libraries. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add missing linker flags for MinGW build
Hello! > If it works, feel free to close the BZ or reply if it doesn't. It doesn't seem to work. And the error is, indeed: --- cut --- CCLD virnetserverclientmock.la .libs/virnetserverclientmock_la-virnetserverclientmock.o: In function `virNetSocketGetSELinuxContext': C:\mingw64\msys\1.0\src\libvirt\tests/virnetserverclientmock.c:61: undefined reference to `rpl_strdup' .libs/virnetserverclientmock_la-virnetserverclientmock.o: In function `virGetUserName': C:\mingw64\msys\1.0\src\libvirt\tests/virnetserverclientmock.c:50: undefined reference to `rpl_strdup' .libs/virnetserverclientmock_la-virnetserverclientmock.o: In function `virGetGroupName': C:\mingw64\msys\1.0\src\libvirt\tests/virnetserverclientmock.c:55: undefined reference to `rpl_strdup' collect2.exe: error: ld returned 1 exit status --- cut --- I will recheck. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia > -Original Message- > From: libvir-list-boun...@redhat.com [mailto:libvir-list- > boun...@redhat.com] On Behalf Of Martin Kletzander > Sent: Monday, April 27, 2015 6:07 PM > To: Pavel Fedin > Cc: libvir-list@redhat.com > Subject: Re: [libvirt] [PATCH] Add missing linker flags for MinGW build > > On Mon, Apr 27, 2015 at 05:40:28PM +0300, Pavel Fedin wrote: > > Hello! > > > >> Thirdly, I wonder why you needed to add this, is gnulib really > needed? > >> is that because of those strdup()s in the file? Why doesn't it fail > >> with gcc then? > >> > >> Fourthly (is that even a word?), I'd ACK this and push it, but just > >> please let me know whether GNULIB_LIBS is really needed here, so I > >> know if I need to amend this or not. > > > > Yes, it is. Without GNULIB i get "undefined symbol" on... > rtl_something... > >Sorry, don't remember, but this clearly belongs to gnulib. > >On Linux .so module can contain undefined symbols, which will be > picked > >up from surrounding binaries; on Windows it cannot. Hence this little > problem. > > > > I already saw that with one build using MinGW (unfortunately I'm > currently unable to use mingw compiler on my machine), so I sent > another patch for that, added your SoB, but added it to LDADD instead > of LDFLAGS as that's the right place to use it. > > > Have a nice day, > Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list