Re: [libvirt] [PATCHv2] qemu: blockCopy: Allow reuse of raw image for shallow block copy

2015-04-28 Thread Eric Blake
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

2015-04-28 Thread Eric Blake
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

2015-04-28 Thread Eric Blake
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

2015-04-28 Thread Eric Blake
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Jim Fehlig
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

2015-04-28 Thread Pavel Boldin
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

2015-04-28 Thread Jiri Denemark
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-28 Thread Vasiliy Tolstov
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

2015-04-28 Thread Vasiliy Tolstov
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

2015-04-28 Thread Vasiliy Tolstov
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Dmitry Guryanov

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

2015-04-28 Thread Dmitry Guryanov

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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Peter Krempa
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.

2015-04-28 Thread Andrea Bolognani
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread John Ferlan


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?

2015-04-28 Thread roky

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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Cole Robinson
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

2015-04-28 Thread Pavel Hrdina
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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)

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread John Ferlan


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

2015-04-28 Thread John Ferlan


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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Nehal J Wani
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

2015-04-28 Thread Martin Kletzander

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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread JĂ¡n Tomko
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread JĂ¡n Tomko
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

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread John Ferlan
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

2015-04-28 Thread John Ferlan


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

2015-04-28 Thread Martin Kletzander
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

2015-04-28 Thread Martin Kletzander

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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread John Ferlan
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

2015-04-28 Thread John Ferlan
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

2015-04-28 Thread John Ferlan
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

2015-04-28 Thread John Ferlan
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

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread Daniel P. Berrange
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Marc-André Lureau
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

2015-04-28 Thread Michal Privoznik
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

2015-04-28 Thread Pavel Boldin
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Pavel Fedin
 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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Roman Bogorodskiy
  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

2015-04-28 Thread Peter Krempa
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

2015-04-28 Thread Pavel Fedin
 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

2015-04-28 Thread Pavel Fedin
 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