Re: [libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def

2015-07-08 Thread lhuang


On 07/09/2015 02:09 PM, Martin Kletzander wrote:

On Thu, Jul 09, 2015 at 11:44:30AM +0800, lhuang wrote:


On 07/08/2015 08:14 PM, Martin Kletzander wrote:

On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:

The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c   | 61

src/conf/domain_conf.h   |  7 ++
src/libvirt_private.syms |  3 +++
3 files changed, 71 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
}


+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name))
+ continue;
+


I think that you shouldn't be able to have two  elements in
the same domain, and since the name is mandatory, STREQ() should be
enough to check whether you need to return current 'i'.



Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one
guest could have more than one  element, maybe one is
non-server shmem and another is server shmem, it depends on how to use
it.



Maybe this function could have a bool parameter (something like a
'full_match') that would say whether everything must match or the name
is enough.  And it the bool is false, streq is enough, but if it's
true, you could abstract the internals of this for body into
virDomainShmemEquals() or something and that would be called instead.



Good idea, this way is okay to me.

Thanks a lot for your opinion.

Luyao


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def

2015-07-08 Thread Martin Kletzander

On Thu, Jul 09, 2015 at 11:44:30AM +0800, lhuang wrote:


On 07/08/2015 08:14 PM, Martin Kletzander wrote:

On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:

The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c   | 61

src/conf/domain_conf.h   |  7 ++
src/libvirt_private.syms |  3 +++
3 files changed, 71 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
}


+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name))
+ continue;
+


I think that you shouldn't be able to have two  elements in
the same domain, and since the name is mandatory, STREQ() should be
enough to check whether you need to return current 'i'.



Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one
guest could have more than one  element, maybe one is
non-server shmem and another is server shmem, it depends on how to use
it.



Maybe this function could have a bool parameter (something like a
'full_match') that would say whether everything must match or the name
is enough.  And it the bool is false, streq is enough, but if it's
true, you could abstract the internals of this for body into
virDomainShmemEquals() or something and that would be called instead.


Well, depending on whether you are unplugging it (you want everything
that was specified to match) or plugging it in (if there's the same
name, just reject it).



Okay, we could just check the name if use the same shmem (both server
and non-server) in the same guest is a invalid case.

Thanks a lot for your review.

Luyao


+ if (shmem->size != tmpshmem->size)
+ continue;
+
+ if (shmem->server.enabled != tmpshmem->server.enabled ||
+ (shmem->server.enabled &&
+ STRNEQ_NULLABLE(shmem->server.chr.data.nix.path,
+ tmpshmem->server.chr.data.nix.path)))
+ continue;
+
+ if (shmem->msi.enabled != tmpshmem->msi.enabled ||
+ (shmem->msi.enabled &&
+  (shmem->msi.vectors != tmpshmem->msi.vectors ||
+   shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd)))
+ continue;
+
+if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ !virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info))
+continue;
+
+break;
+}
+
+if (i < def->nshmems)
+return i;
+
+return -1;
+}
+
+
+virDomainShmemDefPtr
+virDomainShmemRemove(virDomainDefPtr def,
+ size_t idx)
+{
+virDomainShmemDefPtr ret = def->shmems[idx];
+
+VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems);
+
+return ret;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
  virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a4b1bf3..39bc928 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2943,6 +2943,13 @@ int
virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
virDomainMemoryDefPtr mem)
   ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;

+int virDomainShmemInsert(virDomainDefPtr def,
virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ssize_t virDomainShmemFind(virDomainDefPtr def,
virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def,
size_t idx)
+ATTRIBUTE_NONNULL(1);
+
VIR_ENUM_DECL(virDomainTaint)
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3ceb4e3..6127f51 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,9 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
virDomainShutdownReasonTypeFromString;
virDomainShutdownReasonTypeToString;
virDomainShutoffReasonTypeFromString;
--
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list




signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def

2015-07-08 Thread lhuang


On 07/08/2015 08:14 PM, Martin Kletzander wrote:

On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:

The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c   | 61 


src/conf/domain_conf.h   |  7 ++
src/libvirt_private.syms |  3 +++
3 files changed, 71 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
}


+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name))
+ continue;
+


I think that you shouldn't be able to have two  elements in
the same domain, and since the name is mandatory, STREQ() should be
enough to check whether you need to return current 'i'.



Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one 
guest could have more than one  element, maybe one is non-server 
shmem and another is server shmem, it depends on how to use it.



Well, depending on whether you are unplugging it (you want everything
that was specified to match) or plugging it in (if there's the same
name, just reject it).



Okay, we could just check the name if use the same shmem (both server 
and non-server) in the same guest is a invalid case.


Thanks a lot for your review.

Luyao


+ if (shmem->size != tmpshmem->size)
+ continue;
+
+ if (shmem->server.enabled != tmpshmem->server.enabled ||
+ (shmem->server.enabled &&
+ STRNEQ_NULLABLE(shmem->server.chr.data.nix.path,
+ tmpshmem->server.chr.data.nix.path)))
+ continue;
+
+ if (shmem->msi.enabled != tmpshmem->msi.enabled ||
+ (shmem->msi.enabled &&
+  (shmem->msi.vectors != tmpshmem->msi.vectors ||
+   shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd)))
+ continue;
+
+if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ !virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info))
+continue;
+
+break;
+}
+
+if (i < def->nshmems)
+return i;
+
+return -1;
+}
+
+
+virDomainShmemDefPtr
+virDomainShmemRemove(virDomainDefPtr def,
+ size_t idx)
+{
+virDomainShmemDefPtr ret = def->shmems[idx];
+
+VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems);
+
+return ret;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
   virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a4b1bf3..39bc928 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2943,6 +2943,13 @@ int 
virDomainMemoryFindInactiveByDef(virDomainDefPtr def,

 virDomainMemoryDefPtr mem)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;

+int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr 
shmem)

+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr 
shmem)

+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, 
size_t idx)

+ATTRIBUTE_NONNULL(1);
+
VIR_ENUM_DECL(virDomainTaint)
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3ceb4e3..6127f51 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,9 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
virDomainShutdownReasonTypeFromString;
virDomainShutdownReasonTypeToString;
virDomainShutoffReasonTypeFromString;
--
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def

2015-07-08 Thread Martin Kletzander

On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:

The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c   | 61 
src/conf/domain_conf.h   |  7 ++
src/libvirt_private.syms |  3 +++
3 files changed, 71 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
}


+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name))
+ continue;
+


I think that you shouldn't be able to have two  elements in
the same domain, and since the name is mandatory, STREQ() should be
enough to check whether you need to return current 'i'.

Well, depending on whether you are unplugging it (you want everything
that was specified to match) or plugging it in (if there's the same
name, just reject it).


+ if (shmem->size != tmpshmem->size)
+ continue;
+
+ if (shmem->server.enabled != tmpshmem->server.enabled ||
+ (shmem->server.enabled &&
+  STRNEQ_NULLABLE(shmem->server.chr.data.nix.path,
+  tmpshmem->server.chr.data.nix.path)))
+ continue;
+
+ if (shmem->msi.enabled != tmpshmem->msi.enabled ||
+ (shmem->msi.enabled &&
+  (shmem->msi.vectors != tmpshmem->msi.vectors ||
+   shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd)))
+ continue;
+
+if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+!virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info))
+continue;
+
+break;
+}
+
+if (i < def->nshmems)
+return i;
+
+return -1;
+}
+
+
+virDomainShmemDefPtr
+virDomainShmemRemove(virDomainDefPtr def,
+ size_t idx)
+{
+virDomainShmemDefPtr ret = def->shmems[idx];
+
+VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems);
+
+return ret;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
   virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a4b1bf3..39bc928 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2943,6 +2943,13 @@ int virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
 virDomainMemoryDefPtr mem)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;

+int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, size_t idx)
+ATTRIBUTE_NONNULL(1);
+
VIR_ENUM_DECL(virDomainTaint)
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3ceb4e3..6127f51 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,9 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
virDomainShutdownReasonTypeFromString;
virDomainShutdownReasonTypeToString;
virDomainShutoffReasonTypeFromString;
--
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/10] conf: Add helpers to insert/remove/find shmem devices in domain def

2015-06-16 Thread Luyao Huang
The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.

Signed-off-by: Luyao Huang 
---
 src/conf/domain_conf.c   | 61 
 src/conf/domain_conf.h   |  7 ++
 src/libvirt_private.syms |  3 +++
 3 files changed, 71 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 306b718..8a8e4f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def,
 }
 
 
+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
+}
+
+
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem)
+{
+size_t i;
+
+for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name))
+ continue;
+
+ if (shmem->size != tmpshmem->size)
+ continue;
+
+ if (shmem->server.enabled != tmpshmem->server.enabled ||
+ (shmem->server.enabled &&
+  STRNEQ_NULLABLE(shmem->server.chr.data.nix.path,
+  tmpshmem->server.chr.data.nix.path)))
+ continue;
+
+ if (shmem->msi.enabled != tmpshmem->msi.enabled ||
+ (shmem->msi.enabled &&
+  (shmem->msi.vectors != tmpshmem->msi.vectors ||
+   shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd)))
+ continue;
+
+if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+!virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info))
+continue;
+
+break;
+}
+
+if (i < def->nshmems)
+return i;
+
+return -1;
+}
+
+
+virDomainShmemDefPtr
+virDomainShmemRemove(virDomainDefPtr def,
+ size_t idx)
+{
+virDomainShmemDefPtr ret = def->shmems[idx];
+
+VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems);
+
+return ret;
+}
+
+
 char *
 virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a4b1bf3..39bc928 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2943,6 +2943,13 @@ int virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
  virDomainMemoryDefPtr mem)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
+int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, size_t idx)
+ATTRIBUTE_NONNULL(1);
+
 VIR_ENUM_DECL(virDomainTaint)
 VIR_ENUM_DECL(virDomainVirt)
 VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3ceb4e3..6127f51 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,9 @@ virDomainSaveStatus;
 virDomainSaveXML;
 virDomainSeclabelTypeFromString;
 virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
 virDomainShutdownReasonTypeFromString;
 virDomainShutdownReasonTypeToString;
 virDomainShutoffReasonTypeFromString;
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list