3:06 a.m.
v1:
https://www.redhat.com/archives/libvir-list/2015-June/msg00830.html
These patches implement the hot-plug/hot-unplug
and cold-plug/cold-unplug for Inter-VM Shared Memory PCI device.
Luyao Huang (4):
conf: Add helpers to insert/remove/find shmem devices in domain def
qemu: Implement shared memory device cold (un)plug
qemu: Implement share memory device hot-plug
qemu: Implement shared memory device hot-unplug
src/conf/domain_conf.c | 66 ++++++++++++++++++++
src/conf/domain_conf.h | 7 +++
src/libvirt_private.syms | 4 ++
src/qemu/qemu_driver.c | 35 +++++++++--
src/qemu/qemu_hotplug.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 6 ++
6 files changed, 265 insertions(+), 5 deletions(-)
--
1.8.3.1
3:06 a.m.
New subject: [libvirt] [PATCHv2 1/4] conf: Add helpers to insert/remove/find shmem devices in domain def
The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 7 +++++
src/libvirt_private.syms | 3 +++
3 files changed, 76 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 616bf80..cc2a767 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13950,6 +13950,72 @@ 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,
+ bool fullmatch)
+{
+ size_t i;
+
+ for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STREQ(shmem->name, tmpshmem->name)) {
+ if (!fullmatch)
+ return i;
+ } else {
+ 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 8d43ee6..ee76e3f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3012,6 +3012,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, bool
fullmatch)
+ 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 7e60d87..88c2c53 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -451,6 +451,9 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
virDomainShutdownReasonTypeFromString;
virDomainShutdownReasonTypeToString;
virDomainShutoffReasonTypeFromString;
--
1.8.3.1
6:50 p.m.
New subject: [libvirt] [PATCHv2 1/4] conf: Add helpers to insert/remove/find shmem devices in domain def
On 11/26/2015 04:06 AM, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of
shared memory devices.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 7 +++++
src/libvirt_private.syms | 3 +++
3 files changed, 76 insertions(+)
Although there is a v1, it doesn't seem patches 6-10 of that series
(e.g. these patches) were reviewed. Is that correct? Just want to make
sure I'm not missing something from someone else's review...
anyway...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 616bf80..cc2a767 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13950,6 +13950,72 @@ virDomainMemoryRemove(virDomainDefPtr def,
}
+int
+virDomainShmemInsert(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
I see you've followed the virDomainHostdevInsert model...
+{
+ return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
+}
+
+
Would be nice to have an intro comment explaining the params (for other
two as well), but this one is a bit more "interesting" because of arg3.
This includes adding return value.
+ssize_t
+virDomainShmemFind(virDomainDefPtr def,
More or less follows virDomainHostdevFind, but could also be more
explicit using "FindIndex" or "FindIdx"... Not that important.
+ virDomainShmemDefPtr shmem,
+ bool fullmatch)
I think this is more "match name only". It could also be an "unsigned
int flags" where the flags is an enum that could dictate the level of
match required by the caller (perhaps overkill for what is necessary,
but flags just seems to be a better option in my opinion. A passed
value of 0 for flags would indicate to match everything
Beyond that, following virDomainHostdevFind - you could perhaps return
an int. We know size_t will be 0 to def->nshmems...
+{
+ size_t i;
+
+ for (i = 0; i < def->nshmems; i++) {
+ virDomainShmemDefPtr tmpshmem = def->shmems[i];
+
+ if (STREQ(shmem->name, tmpshmem->name)) {
+ if (!fullmatch)
+ return i;
+ } else {
+ continue;
+ }
Perhaps cleaner as:
if (STRNEQ(shmem->name, tmpshmem->name))
continue;
if (!fullmatch)
return i;
You could also use matchnameonly or (flags &
VIR_DOMAIN_SHMEM_NAME_MATCH_ONLY)
+
+ if (shmem->size != tmpshmem->size)
+ continue;
+
Eventually someone could add other flags such as
-> match name and size only...
-> match name, size, and server
-> match name, size, server, and msi
-> match name, size, server, msi, and info/address
+ 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;
Matching address is such a touchy thing, I know why it's here to be
complete, but since the remove device doesn't have to add it to the XML
passed in, is there a need? I guess we can leave it for now unless
someone else offers up a different opinion.
+
+ break;
Why not just return i; here?
+ }
+
+ if (i < def->nshmems)
+ return i;
Removing need for this check
+
+ 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 8d43ee6..ee76e3f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3012,6 +3012,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, bool
fullmatch)
+ 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 7e60d87..88c2c53 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -451,6 +451,9 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemFind;
+virDomainShmemInsert;
+virDomainShmemRemove;
virDomainShutdownReasonTypeFromString;
virDomainShutdownReasonTypeToString;
virDomainShutoffReasonTypeFromString;
2:25 a.m.
New subject: [libvirt] [PATCHv2 1/4] conf: Add helpers to insert/remove/find shmem devices in domain def
On 12/10/2015 08:50 AM, John Ferlan wrote:
On 11/26/2015 04:06 AM, Luyao Huang wrote:
> The helpers will be useful when implementing hotplug and coldplug of
> shared memory devices.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 7 +++++
> src/libvirt_private.syms | 3 +++
> 3 files changed, 76 insertions(+)
>
Although there is a v1, it doesn't seem patches 6-10 of that series
(e.g. these patches) were reviewed. Is that correct? Just want to make
sure I'm not missing something from someone else's review...
anyway...
here is the review of patch 10:
https://www.redhat.com/archives/libvir-list/2015-July/msg00338.html
patch 6:
https://www.redhat.com/archives/libvir-list/2015-July/msg00319.html
And patch 7-9 were not reviewed, i posted a new version since i
introduced a new parameter in virDomainShmemFind() and removed audit
part (I remember Martin will help finish that part, but i am sorry that
i forgot mention this in some place)
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 616bf80..cc2a767 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13950,6 +13950,72 @@ virDomainMemoryRemove(virDomainDefPtr def,
> }
>
>
> +int
> +virDomainShmemInsert(virDomainDefPtr def,
> + virDomainShmemDefPtr shmem)
I see you've followed the virDomainHostdevInsert model...
> +{
> + return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem);
> +}
> +
> +
Would be nice to have an intro comment explaining the params (for other
two as well), but this one is a bit more "interesting" because of arg3.
This includes adding return value.
Okay, i will add them in next version.
> +ssize_t
> +virDomainShmemFind(virDomainDefPtr def,
More or less follows virDomainHostdevFind, but could also be more
explicit using "FindIndex" or "FindIdx"... Not that important.
> + virDomainShmemDefPtr shmem,
> + bool fullmatch)
I think this is more "match name only". It could also be an "unsigned
int flags" where the flags is an enum that could dictate the level of
match required by the caller (perhaps overkill for what is necessary,
but flags just seems to be a better option in my opinion. A passed
value of 0 for flags would indicate to match everything
interesting idea, i will have a try with your option, you will see it in
next version :)
Beyond that, following virDomainHostdevFind - you could perhaps
return
an int. We know size_t will be 0 to def->nshmems...
got it.
> +{
> + size_t i;
> +
> + for (i = 0; i < def->nshmems; i++) {
> + virDomainShmemDefPtr tmpshmem = def->shmems[i];
> +
> + if (STREQ(shmem->name, tmpshmem->name)) {
> + if (!fullmatch)
> + return i;
> + } else {
> + continue;
> + }
Perhaps cleaner as:
if (STRNEQ(shmem->name, tmpshmem->name))
continue;
if (!fullmatch)
return i;
You could also use matchnameonly or (flags &
VIR_DOMAIN_SHMEM_NAME_MATCH_ONLY)
Okay
> +
> + if (shmem->size != tmpshmem->size)
> + continue;
> +
Eventually someone could add other flags such as
-> match name and size only...
-> match name, size, and server
-> match name, size, server, and msi
-> match name, size, server, msi, and info/address
I see.
> + 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;
Matching address is such a touchy thing, I know why it's here to be
complete, but since the remove device doesn't have to add it to the XML
passed in, is there a need? I guess we can leave it for now unless
someone else offers up a different opinion.
Indeed, i will remove this check in next version.
> +
> + break;
Why not just return i; here?
You are right, i will fix this and ...
> + }
> +
> + if (i < def->nshmems)
> + return i;
Removing need for this check
... this in next version
Thanks a lot for your reviews and suggestions.
Luyao
> +
> + 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 8d43ee6..ee76e3f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3012,6 +3012,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, bool
fullmatch)
> + 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 7e60d87..88c2c53 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -451,6 +451,9 @@ virDomainSaveStatus;
> virDomainSaveXML;
> virDomainSeclabelTypeFromString;
> virDomainSeclabelTypeToString;
> +virDomainShmemFind;
> +virDomainShmemInsert;
> +virDomainShmemRemove;
> virDomainShutdownReasonTypeFromString;
> virDomainShutdownReasonTypeToString;
> virDomainShutoffReasonTypeFromString;
>
3:06 a.m.
New subject: [libvirt] [PATCHv2 2/4] qemu: Implement shared memory device cold (un)plug
Add support for using the attach/detach device APIs on the inactive
configuration to add/del shared memory devices.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 21 +++++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 88c2c53..c89d27a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -451,6 +451,7 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemDefFree;
virDomainShmemFind;
virDomainShmemInsert;
virDomainShmemRemove;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 65ccf99..5ded9ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8174,6 +8174,15 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
dev->data.memory = NULL;
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ if (virDomainShmemInsert(vmdef, dev->data.shmem) < 0)
+ return -1;
+ dev->data.shmem = NULL;
+
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ return -1;
+ break;
+
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
@@ -8183,7 +8192,6 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
@@ -8310,6 +8318,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx));
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ if ((idx = virDomainShmemFind(vmdef, dev->data.shmem, true)) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("no matching shared memory device was found"));
+ return -1;
+ }
+
+ virDomainShmemDefFree(virDomainShmemRemove(vmdef, idx));
+ break;
+
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
@@ -8319,7 +8337,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
--
1.8.3.1
6:53 p.m.
New subject: [libvirt] [PATCHv2 2/4] qemu: Implement shared memory device cold (un)plug
On 11/26/2015 04:06 AM, Luyao Huang wrote:
Add support for using the attach/detach device APIs on the inactive
configuration to add/del shared memory devices.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 21 +++++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 88c2c53..c89d27a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -451,6 +451,7 @@ virDomainSaveStatus;
virDomainSaveXML;
virDomainSeclabelTypeFromString;
virDomainSeclabelTypeToString;
+virDomainShmemDefFree;
virDomainShmemFind;
virDomainShmemInsert;
virDomainShmemRemove;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 65ccf99..5ded9ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8174,6 +8174,15 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
dev->data.memory = NULL;
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ if (virDomainShmemInsert(vmdef, dev->data.shmem) < 0)
+ return -1;
+ dev->data.shmem = NULL;
+
+ if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
+ return -1;
+ break;
+
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
@@ -8183,7 +8192,6 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
@@ -8310,6 +8318,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx));
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ if ((idx = virDomainShmemFind(vmdef, dev->data.shmem, true)) < 0) {
Here rather than 'true' which to me only has meaning if I go read the
code... Of course same thing holds true when passing a 0 (zero) flags value.
BTW: My idea around flags matches what I recently added for
VolReadErrorMode in storage_backend.h... But for this it'd be something
like the following in (I assume) domain_conf.h near _virDomainShmemDef.
/* DomainShmemMatchFlags
* Flags to dictate the level of matching when searching for a
* shmem object in the domain 'nshmems' list.
*/
enum {
VIR_DOMAIN_SHMEM_MATCH_NAME_ONLY = 1 << 0,
}
In general though it seems fine.
John
+ virReportError(VIR_ERR_OPERATION_FAILED,
"%s",
+ _("no matching shared memory device was found"));
+ return -1;
+ }
+
+ virDomainShmemDefFree(virDomainShmemRemove(vmdef, idx));
+ break;
+
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
@@ -8319,7 +8337,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
2:45 a.m.
New subject: [libvirt] [PATCHv2 2/4] qemu: Implement shared memory device cold (un)plug
On 12/10/2015 08:53 AM, John Ferlan wrote:
On 11/26/2015 04:06 AM, Luyao Huang wrote:
> Add support for using the attach/detach device APIs on the inactive
> configuration to add/del shared memory devices.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 21 +++++++++++++++++++--
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 88c2c53..c89d27a 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -451,6 +451,7 @@ virDomainSaveStatus;
> virDomainSaveXML;
> virDomainSeclabelTypeFromString;
> virDomainSeclabelTypeToString;
> +virDomainShmemDefFree;
> virDomainShmemFind;
> virDomainShmemInsert;
> virDomainShmemRemove;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 65ccf99..5ded9ef 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8174,6 +8174,15 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
> dev->data.memory = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_SHMEM:
> + if (virDomainShmemInsert(vmdef, dev->data.shmem) < 0)
> + return -1;
> + dev->data.shmem = NULL;
> +
> + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
> + return -1;
> + break;
> +
> case VIR_DOMAIN_DEVICE_INPUT:
> case VIR_DOMAIN_DEVICE_SOUND:
> case VIR_DOMAIN_DEVICE_VIDEO:
> @@ -8183,7 +8192,6 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
> case VIR_DOMAIN_DEVICE_SMARTCARD:
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> case VIR_DOMAIN_DEVICE_NVRAM:
> - case VIR_DOMAIN_DEVICE_SHMEM:
> case VIR_DOMAIN_DEVICE_REDIRDEV:
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_TPM:
> @@ -8310,6 +8318,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
> virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx));
> break;
>
> + case VIR_DOMAIN_DEVICE_SHMEM:
> + if ((idx = virDomainShmemFind(vmdef, dev->data.shmem, true)) < 0) {
Here rather than 'true' which to me only has meaning if I go read the
code... Of course same thing holds true when passing a 0 (zero) flags value.
BTW: My idea around flags matches what I recently added for
VolReadErrorMode in storage_backend.h... But for this it'd be something
like the following in (I assume) domain_conf.h near _virDomainShmemDef.
/* DomainShmemMatchFlags
* Flags to dictate the level of matching when searching for a
* shmem object in the domain 'nshmems' list.
*/
enum {
VIR_DOMAIN_SHMEM_MATCH_NAME_ONLY = 1 << 0,
}
In general though it seems fine.
Okay, i have saw your comment mentioned this in another mail, and i will
post a new version include your idea.
Thanks a lot for your review and suggestion.
Luyao
John
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("no matching shared memory device was
found"));
> + return -1;
> + }
> +
> + virDomainShmemDefFree(virDomainShmemRemove(vmdef, idx));
> + break;
> +
> case VIR_DOMAIN_DEVICE_INPUT:
> case VIR_DOMAIN_DEVICE_SOUND:
> case VIR_DOMAIN_DEVICE_VIDEO:
> @@ -8319,7 +8337,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
> case VIR_DOMAIN_DEVICE_SMARTCARD:
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> case VIR_DOMAIN_DEVICE_NVRAM:
> - case VIR_DOMAIN_DEVICE_SHMEM:
> case VIR_DOMAIN_DEVICE_REDIRDEV:
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_TPM:
>
3:06 a.m.
New subject: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_driver.c | 10 ++++++++-
src/qemu/qemu_hotplug.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 +++
3 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ded9ef..3c25c07 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7784,6 +7784,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
dev->data.memory = NULL;
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ ret = qemuDomainAttachShmemDevice(driver, vm,
+ dev->data.shmem);
+ if (!ret) {
+ alias = dev->data.shmem->info.alias;
+ dev->data.shmem = NULL;
+ }
+ break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -7795,7 +7804,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
case VIR_DOMAIN_DEVICE_LAST:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8804d3d..c5e544d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1882,6 +1882,64 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
}
+int
+qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ char *devstr = NULL;
+ char *charAlias = NULL;
+
+ if (virAsprintf(&shmem->info.alias, "shmem%zu",
vm->def->nshmems) < 0)
+ return -1;
+
+ if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
+ return -1;
+
+ if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+ (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &shmem->info) < 0))
+ return -1;
+
+ if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
+ goto cleanup;
+
+ if (virAsprintf(&charAlias, "char%s", shmem->info.alias) < 0)
+ goto cleanup;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (shmem->server.enabled &&
+ qemuMonitorAttachCharDev(priv->mon, charAlias,
+ &shmem->server.chr) < 0) {
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ goto cleanup;
+ }
+
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
+ if (shmem->server.enabled)
+ ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ goto cleanup;
+ }
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ if (virDomainShmemInsert(vm->def, shmem) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ if (ret < 0)
+ qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
+ VIR_FREE(charAlias);
+ VIR_FREE(devstr);
+ return ret;
+}
+
+
static int
qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 4140da3..60137a6 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainRNGDefPtr rng);
+int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem);
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
--
1.8.3.1
7:23 p.m.
New subject: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug
$SUBJ
s/share/shared
On 11/26/2015 04:06 AM, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_driver.c | 10 ++++++++-
src/qemu/qemu_hotplug.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 3 +++
3 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ded9ef..3c25c07 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7784,6 +7784,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
dev->data.memory = NULL;
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ ret = qemuDomainAttachShmemDevice(driver, vm,
+ dev->data.shmem);
+ if (!ret) {
+ alias = dev->data.shmem->info.alias;
+ dev->data.shmem = NULL;
+ }
+ break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -7795,7 +7804,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
case VIR_DOMAIN_DEVICE_LAST:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8804d3d..c5e544d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1882,6 +1882,64 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
}
It seems this is modeled after qemuDomainAttachRNGDevice, right?
+int
+qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ char *devstr = NULL;
+ char *charAlias = NULL;
+
+ if (virAsprintf(&shmem->info.alias, "shmem%zu",
vm->def->nshmems) < 0)
+ return -1;
+
+ if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
+ return -1;
+
+ if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+ (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &shmem->info) <
0))
+ return -1;
+
+ if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
+ goto cleanup;
+
+ if (virAsprintf(&charAlias, "char%s",
shmem->info.alias) < 0)
+ goto cleanup;
+
This seems to be up to a 2 stage process "if" server.enabled" is
true"...
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (shmem->server.enabled &&
+ qemuMonitorAttachCharDev(priv->mon, charAlias,
+ &shmem->server.chr) < 0) {
Instead of the following change to:
goto failchardev;
and the {} won't be necessary
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ goto cleanup;
+ }
+
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
Instead of the following change to
goto failadddevice;
and the {} won't be necessary
+ if (shmem->server.enabled)
+ ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ goto cleanup;
+ }
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
Again following RNG model - this should have a
if (*Exit*() < 0) {
vm = NULL;
goto cleanup
}
+
+ if (virDomainShmemInsert(vm->def, shmem) < 0)
+ goto cleanup;
And of course the auditing change as well.
+
+ ret = 0;
+ cleanup:
+ if (ret < 0)
Following RNG the && vm would be used here... See your patch commits
'0ed3b3353' and '980b265d0'
+ qemuDomainReleaseDeviceAddress(vm, &shmem->info,
NULL);
+ VIR_FREE(charAlias);
+ VIR_FREE(devstr);
+ return ret;
failadddevice:
if (shmem->server.enabled)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
failchardev:
ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
Hope this all makes sense (it's been a long day ;-))
John
+}
+
+
static int
qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 4140da3..60137a6 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainRNGDefPtr rng);
+int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem);
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
3:24 a.m.
New subject: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug
On 12/10/2015 09:23 AM, John Ferlan wrote:
$SUBJ
s/share/shared
thanks for pointing out this
On 11/26/2015 04:06 AM, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 10 ++++++++-
> src/qemu/qemu_hotplug.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_hotplug.h | 3 +++
> 3 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5ded9ef..3c25c07 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7784,6 +7784,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> dev->data.memory = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_SHMEM:
> + ret = qemuDomainAttachShmemDevice(driver, vm,
> + dev->data.shmem);
> + if (!ret) {
> + alias = dev->data.shmem->info.alias;
> + dev->data.shmem = NULL;
> + }
> + break;
> +
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_FS:
> case VIR_DOMAIN_DEVICE_INPUT:
> @@ -7795,7 +7804,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> case VIR_DOMAIN_DEVICE_SMARTCARD:
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> case VIR_DOMAIN_DEVICE_NVRAM:
> - case VIR_DOMAIN_DEVICE_SHMEM:
> case VIR_DOMAIN_DEVICE_TPM:
> case VIR_DOMAIN_DEVICE_PANIC:
> case VIR_DOMAIN_DEVICE_LAST:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8804d3d..c5e544d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1882,6 +1882,64 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
> }
>
>
It seems this is modeled after qemuDomainAttachRNGDevice, right?
Right :)
> +int
> +qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + int ret = -1;
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + char *devstr = NULL;
> + char *charAlias = NULL;
> +
> + if (virAsprintf(&shmem->info.alias, "shmem%zu",
vm->def->nshmems) < 0)
> + return -1;
> +
> + if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
> + return -1;
> +
> + if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
> + shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
> + (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &shmem->info) <
0))
> + return -1;
> +
> + if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
> + goto cleanup;
> +
> + if (virAsprintf(&charAlias, "char%s", shmem->info.alias) <
0)
> + goto cleanup;
> +
This seems to be up to a 2 stage process "if" server.enabled" is
true"....
Indeed, i will fix it in next version
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + if (shmem->server.enabled &&
> + qemuMonitorAttachCharDev(priv->mon, charAlias,
> + &shmem->server.chr) < 0) {
Instead of the following change to:
goto failchardev;
and the {} won't be necessary
...
Instead of the following change to
goto failadddevice;
and the {} won't be necessary
...
Again following RNG model - this should have a
if (*Exit*() < 0) {
vm = NULL;
goto cleanup
}
...
And of course the auditing change as well.
...
Following RNG the && vm would be used here... See your patch
commits
'0ed3b3353' and '980b265d0'
...
failadddevice:
if (shmem->server.enabled)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
failchardev:
ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
Nice improve, i must forgot a old problem that the &vm will be changed
after qemu unexpected exit during enter the monitor.
Hope this all makes sense (it's been a long day ;-))
Thanks a lot for your review and suggestion, and i am sorry that i
didn't reply your mails when i saw them (I'm kind of overwhelmed lately :) )
Have a nice day !
Luyao
John
> +}
> +
> +
> static int
> qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index 4140da3..60137a6 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
> int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainRNGDefPtr rng);
> +int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem);
>
> int
> qemuDomainChrInsert(virDomainDefPtr vmdef,
>
7:26 p.m.
New subject: [libvirt] [PATCHv2 3/4] qemu: Implement share memory device hot-plug
[...]
+int
+qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ char *devstr = NULL;
+ char *charAlias = NULL;
Dang... one more thing... why no?
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("qemu does not support -device"));
return -1;
}
I see it's not in AttachRNG (so why it was missed), *BUT* it is in
DetachRNG... So it seems a separate patch for AttachRNG should add that?
John
3:07 a.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++-
src/qemu/qemu_hotplug.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 3 ++
3 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3c25c07..9e7429a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7881,6 +7881,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_MEMORY:
ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
+ break;
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -7892,7 +7895,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c5e544d..4ab05f3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3347,6 +3347,45 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
}
+static int
+qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ virObjectEventPtr event;
+ char *charAlias = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ ssize_t idx;
+ int rc = 0;
+
+ VIR_DEBUG("Removing shared memory device %s from domain %p %s",
+ shmem->info.alias, vm, vm->def->name);
+
+ if (shmem->server.enabled) {
+ if (virAsprintf(&charAlias, "char%s", shmem->info.alias) <
0)
+ return -1;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
+ VIR_FREE(charAlias);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ return -1;
+ }
+
+ if (rc < 0)
+ return -1;
+
+ if ((event = virDomainEventDeviceRemovedNewFromObj(vm, shmem->info.alias)))
+ qemuDomainEventQueue(driver, event);
+
+ if ((idx = virDomainShmemFind(vm->def, shmem, true)) >= 0)
+ virDomainShmemRemove(vm->def, idx);
+ qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
+ virDomainShmemDefFree(shmem);
+ return 0;
+}
+
+
int
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -3378,6 +3417,10 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory);
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ ret = qemuDomainRemoveShmemDevice(driver, vm, dev->data.shmem);
+ break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
@@ -3391,7 +3434,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
case VIR_DOMAIN_DEVICE_LAST:
@@ -4378,3 +4420,53 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
qemuDomainResetDeviceRemoval(vm);
return ret;
}
+
+
+int
+qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ ssize_t idx;
+ virDomainShmemDefPtr tmpshmem;
+ int rc;
+ int ret = -1;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("qemu does not support -device"));
+ return -1;
+ }
+
+ if ((idx = virDomainShmemFind(vm->def, shmem, true)) < 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("device not present in domain configuration"));
+ return -1;
+ }
+
+ tmpshmem = vm->def->shmems[idx];
+
+ if (!tmpshmem->info.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("alias not set for shared memory device"));
+ return -1;
+ }
+
+ qemuDomainMarkDeviceForRemoval(vm, &tmpshmem->info);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ rc = qemuMonitorDelDevice(priv->mon, tmpshmem->info.alias);
+ if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
+ goto cleanup;
+
+ rc = qemuDomainWaitForDeviceRemoval(vm);
+ if (rc == 0 || rc == 1)
+ ret = qemuDomainRemoveShmemDevice(driver, vm, tmpshmem);
+ else
+ ret = 0;
+
+ cleanup:
+ qemuDomainResetDeviceRemoval(vm);
+ return ret;
+}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 60137a6..1b18e8a 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -112,6 +112,9 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainShmemDefPtr shmem);
+int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem);
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
--
1.8.3.1
7:43 p.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
On 11/26/2015 04:07 AM, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++-
src/qemu/qemu_hotplug.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 3 ++
3 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3c25c07..9e7429a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7881,6 +7881,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_MEMORY:
ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
+ break;
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -7892,7 +7895,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c5e544d..4ab05f3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3347,6 +3347,45 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
}
+static int
+qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ virObjectEventPtr event;
+ char *charAlias = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ ssize_t idx;
+ int rc = 0;
+
+ VIR_DEBUG("Removing shared memory device %s from domain %p %s",
+ shmem->info.alias, vm, vm->def->name);
+
+ if (shmem->server.enabled) {
+ if (virAsprintf(&charAlias, "char%s", shmem->info.alias) <
0)
+ return -1;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
+ VIR_FREE(charAlias);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ return -1;
+ }
+
Auditing code?
+ if (rc < 0)
+ return -1;
+
I know this is a copy of the RemoveRNGDevice; however, this code doesn't
remove an 'obj'. In fact, if !shmem->server.enabled, then we don't enter
the monitor at all.
Thus the following event probably won't happen...
+ if ((event = virDomainEventDeviceRemovedNewFromObj(vm,
shmem->info.alias)))
+ qemuDomainEventQueue(driver, event);
+
+ if ((idx = virDomainShmemFind(vm->def, shmem, true)) >= 0)
+ virDomainShmemRemove(vm->def, idx);
+ qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
+ virDomainShmemDefFree(shmem);
+ return 0;
+}
+
+
int
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -3378,6 +3417,10 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory);
break;
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ ret = qemuDomainRemoveShmemDevice(driver, vm, dev->data.shmem);
+ break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
@@ -3391,7 +3434,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
- case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
case VIR_DOMAIN_DEVICE_LAST:
@@ -4378,3 +4420,53 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
qemuDomainResetDeviceRemoval(vm);
return ret;
}
+
+
+int
+qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ ssize_t idx;
+ virDomainShmemDefPtr tmpshmem;
+ int rc;
+ int ret = -1;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("qemu does not support -device"));
+ return -1;
+ }
Well it's here, but not in AttachShmemDevice...
+
+ if ((idx = virDomainShmemFind(vm->def, shmem, true)) < 0) {
Again we could use a 'flags' argument instead... Of course there isn't
a "false" argument to any of the added callers to date, so not sure of
the need for it... Could still use/add flags and keep it as "unused"
for now.
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("device not present in domain configuration"));
s/(("/(("shared memory /
John
+ return -1;
+ }
+
+ tmpshmem = vm->def->shmems[idx];
+
+ if (!tmpshmem->info.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("alias not set for shared memory device"));
+ return -1;
+ }
+
+ qemuDomainMarkDeviceForRemoval(vm, &tmpshmem->info);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ rc = qemuMonitorDelDevice(priv->mon, tmpshmem->info.alias);
+ if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
+ goto cleanup;
+
+ rc = qemuDomainWaitForDeviceRemoval(vm);
+ if (rc == 0 || rc == 1)
+ ret = qemuDomainRemoveShmemDevice(driver, vm, tmpshmem);
+ else
+ ret = 0;
+
+ cleanup:
+ qemuDomainResetDeviceRemoval(vm);
+ return ret;
+}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 60137a6..1b18e8a 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -112,6 +112,9 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainShmemDefPtr shmem);
+int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem);
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
4:07 a.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
On 12/10/2015 09:43 AM, John Ferlan wrote:
On 11/26/2015 04:07 AM, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 4 ++-
> src/qemu/qemu_hotplug.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-
> src/qemu/qemu_hotplug.h | 3 ++
> 3 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3c25c07..9e7429a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7881,6 +7881,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
> case VIR_DOMAIN_DEVICE_MEMORY:
> ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
> break;
> + case VIR_DOMAIN_DEVICE_SHMEM:
> + ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
> + break;
>
> case VIR_DOMAIN_DEVICE_FS:
> case VIR_DOMAIN_DEVICE_INPUT:
> @@ -7892,7 +7895,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
> case VIR_DOMAIN_DEVICE_SMARTCARD:
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> case VIR_DOMAIN_DEVICE_NVRAM:
> - case VIR_DOMAIN_DEVICE_SHMEM:
> case VIR_DOMAIN_DEVICE_REDIRDEV:
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_TPM:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c5e544d..4ab05f3 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3347,6 +3347,45 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
> }
>
>
> +static int
> +qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + virObjectEventPtr event;
> + char *charAlias = NULL;
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + ssize_t idx;
> + int rc = 0;
> +
> + VIR_DEBUG("Removing shared memory device %s from domain %p %s",
> + shmem->info.alias, vm, vm->def->name);
> +
> + if (shmem->server.enabled) {
> + if (virAsprintf(&charAlias, "char%s", shmem->info.alias)
< 0)
> + return -1;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> + rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
> + VIR_FREE(charAlias);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + return -1;
> + }
> +
Auditing code?
I have removed them since i remember @Martin will finish the audit part.
> + if (rc < 0)
> + return -1;
> +
I know this is a copy of the RemoveRNGDevice; however, this code doesn't
remove an 'obj'. In fact, if !shmem->server.enabled, then we don't enter
the monitor at all.
Thus the following event probably won't happen...
I am not sure what your mean is ... i guess your mean the device remove
event we get from qmp monitor won't happen ? we will get that event if
qemu remove shmem device success, it should always happen if qemu really
remove it and there is no bugs on qemu :)
> + if ((event = virDomainEventDeviceRemovedNewFromObj(vm,
shmem->info.alias)))
> + qemuDomainEventQueue(driver, event);
> +
> + if ((idx = virDomainShmemFind(vm->def, shmem, true)) >= 0)
> + virDomainShmemRemove(vm->def, idx);
> + qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
> + virDomainShmemDefFree(shmem);
> + return 0;
> +}
> +
> +
> int
> qemuDomainRemoveDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> @@ -3378,6 +3417,10 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
> ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory);
> break;
>
> + case VIR_DOMAIN_DEVICE_SHMEM:
> + ret = qemuDomainRemoveShmemDevice(driver, vm, dev->data.shmem);
> + break;
> +
> case VIR_DOMAIN_DEVICE_NONE:
> case VIR_DOMAIN_DEVICE_LEASE:
> case VIR_DOMAIN_DEVICE_FS:
> @@ -3391,7 +3434,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
> case VIR_DOMAIN_DEVICE_SMARTCARD:
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> case VIR_DOMAIN_DEVICE_NVRAM:
> - case VIR_DOMAIN_DEVICE_SHMEM:
> case VIR_DOMAIN_DEVICE_TPM:
> case VIR_DOMAIN_DEVICE_PANIC:
> case VIR_DOMAIN_DEVICE_LAST:
> @@ -4378,3 +4420,53 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
> qemuDomainResetDeviceRemoval(vm);
> return ret;
> }
> +
> +
> +int
> +qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + ssize_t idx;
> + virDomainShmemDefPtr tmpshmem;
> + int rc;
> + int ret = -1;
> +
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("qemu does not support -device"));
> + return -1;
> + }
Well it's here, but not in AttachShmemDevice...
I must forgot add it in the AttachShmemDevice, thanks for pointing out.
> +
> + if ((idx = virDomainShmemFind(vm->def, shmem, true)) < 0) {
Again we could use a 'flags' argument instead... Of course there isn't
a "false" argument to any of the added callers to date, so not sure of
the need for it... Could still use/add flags and keep it as "unused"
for now.
Okay
> + virReportError(VIR_ERR_OPERATION_INVALID,
"%s",
> + _("device not present in domain configuration"));
s/(("/(("shared memory /
I will fix it in next version,
Thanks a lot for your review
Luyao
John
> + return -1;
> + }
> +
> + tmpshmem = vm->def->shmems[idx];
> +
> + if (!tmpshmem->info.alias) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("alias not set for shared memory device"));
> + return -1;
> + }
> +
> + qemuDomainMarkDeviceForRemoval(vm, &tmpshmem->info);
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> + rc = qemuMonitorDelDevice(priv->mon, tmpshmem->info.alias);
> + if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
> + goto cleanup;
> +
> + rc = qemuDomainWaitForDeviceRemoval(vm);
> + if (rc == 0 || rc == 1)
> + ret = qemuDomainRemoveShmemDevice(driver, vm, tmpshmem);
> + else
> + ret = 0;
> +
> + cleanup:
> + qemuDomainResetDeviceRemoval(vm);
> + return ret;
> +}
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index 60137a6..1b18e8a 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -112,6 +112,9 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
> int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainShmemDefPtr shmem);
> +int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem);
>
> int
> qemuDomainChrInsert(virDomainDefPtr vmdef,
>
6:39 a.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
[...]
>> + if (rc < 0)
>> + return -1;
>> +
> I know this is a copy of the RemoveRNGDevice; however, this code doesn't
> remove an 'obj'. In fact, if !shmem->server.enabled, then we don't
enter
> the monitor at all.
>
> Thus the following event probably won't happen...
I am not sure what your mean is ... i guess your mean the device remove
event we get from qmp monitor won't happen ? we will get that event if
qemu remove shmem device success, it should always happen if qemu really
remove it and there is no bugs on qemu :)
While reviewing I got lazy and didn't check the non hotplug case to how
shmem is added to the vm, but the point I was trying to make is that "if
(shmem->server.enabled)" fails (e.g. is false), then there is no "rc =
qemuMonitorDelObject(priv->mon, objAlias);" call in this API (similar to
RNG code), thus how does the following event get triggered? Even if the
condition was true, does detaching the char dev cause the event to be
triggered?
I thought the event was related to the DelObject code, but I didn't go
follow that code
John
[...]
8:43 p.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
On 12/15/2015 08:39 PM, John Ferlan wrote:
[...]
>>> + if (rc < 0)
>>> + return -1;
>>> +
>> I know this is a copy of the RemoveRNGDevice; however, this code doesn't
>> remove an 'obj'. In fact, if !shmem->server.enabled, then we don't
enter
>> the monitor at all.
>>
>> Thus the following event probably won't happen...
> I am not sure what your mean is ... i guess your mean the device remove
> event we get from qmp monitor won't happen ? we will get that event if
> qemu remove shmem device success, it should always happen if qemu really
> remove it and there is no bugs on qemu :)
>
While reviewing I got lazy and didn't check the non hotplug case to how
shmem is added to the vm, but the point I was trying to make is that "if
(shmem->server.enabled)" fails (e.g. is false), then there is no "rc =
qemuMonitorDelObject(priv->mon, objAlias);" call in this API (similar to
RNG code), thus how does the following event get triggered? Even if the
condition was true, does detaching the char dev cause the event to be
triggered?
I thought the event was related to the DelObject code, but I didn't go
follow that code
Oh, i see, event is not related to the object delete, i guess you have
checked the code and know how it works now ;-)
Thanks for your quick reply !
Luyao
John
[...]
6:26 a.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
On 12/16/2015 09:43 PM, lhuang wrote:
On 12/15/2015 08:39 PM, John Ferlan wrote:
> [...]
>
>>>> + if (rc < 0)
>>>> + return -1;
>>>> +
>>> I know this is a copy of the RemoveRNGDevice; however, this code
>>> doesn't
>>> remove an 'obj'. In fact, if !shmem->server.enabled, then we
don't
>>> enter
>>> the monitor at all.
>>>
>>> Thus the following event probably won't happen...
>> I am not sure what your mean is ... i guess your mean the device remove
>> event we get from qmp monitor won't happen ? we will get that event if
>> qemu remove shmem device success, it should always happen if qemu really
>> remove it and there is no bugs on qemu :)
>>
> While reviewing I got lazy and didn't check the non hotplug case to how
> shmem is added to the vm, but the point I was trying to make is that "if
> (shmem->server.enabled)" fails (e.g. is false), then there is no "rc =
> qemuMonitorDelObject(priv->mon, objAlias);" call in this API (similar to
> RNG code), thus how does the following event get triggered? Even if the
> condition was true, does detaching the char dev cause the event to be
> triggered?
> I thought the event was related to the DelObject code, but I didn't go
> follow that code
Oh, i see, event is not related to the object delete, i guess you have
checked the code and know how it works now ;-)
Maybe you misunderstood - maybe I misunderstood your response... I
didn't go searching through the code, but something just didn't seem
right when looking so I wanted to note it.
Since I figured your model was RNG my thought process was how does RNG
do it and what is different...
AttachRNG has 3 steps AttachCharDev, AddObject, and AddDevice
DetachRNG has 1 step DelDevice
RemoveRNG has 2 steps DelObject and DetachCharDev
AttachShmem has 2 steps AttachCharDev and AddDevice
DetachShmem has 1 step DelDevice
RemoveShmem has 1 step DetachCharDev
I think my concern was more related to the difference where an RNG has
an Object that gets removed, but Shmem doesn't.
After closer inspection now it seems that this event generation needs to
stay. The RemoveShmem code is generating the event not waiting on it
which perhaps is where my thoughts were when I first went through the
changes.
John
Thanks for your quick reply !
Luyao
> John
>
> [...]
>
8:24 p.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
On 12/17/2015 08:26 PM, John Ferlan wrote:
On 12/16/2015 09:43 PM, lhuang wrote:
>
> On 12/15/2015 08:39 PM, John Ferlan wrote:
>> [...]
>>
>>>>> + if (rc < 0)
>>>>> + return -1;
>>>>> +
>>>> I know this is a copy of the RemoveRNGDevice; however, this code
>>>> doesn't
>>>> remove an 'obj'. In fact, if !shmem->server.enabled, then we
don't
>>>> enter
>>>> the monitor at all.
>>>>
>>>> Thus the following event probably won't happen...
>>> I am not sure what your mean is ... i guess your mean the device remove
>>> event we get from qmp monitor won't happen ? we will get that event if
>>> qemu remove shmem device success, it should always happen if qemu really
>>> remove it and there is no bugs on qemu :)
>>>
>> While reviewing I got lazy and didn't check the non hotplug case to how
>> shmem is added to the vm, but the point I was trying to make is that "if
>> (shmem->server.enabled)" fails (e.g. is false), then there is no "rc
=
>> qemuMonitorDelObject(priv->mon, objAlias);" call in this API (similar to
>> RNG code), thus how does the following event get triggered? Even if the
>> condition was true, does detaching the char dev cause the event to be
>> triggered?
>> I thought the event was related to the DelObject code, but I didn't go
>> follow that code
> Oh, i see, event is not related to the object delete, i guess you have
> checked the code and know how it works now ;-)
Maybe you misunderstood - maybe I misunderstood your response... I
didn't go searching through the code, but something just didn't seem
right when looking so I wanted to note it.
Looks like i misunderstood :)
Since I figured your model was RNG my thought process was how does
RNG
do it and what is different...
AttachRNG has 3 steps AttachCharDev, AddObject, and AddDevice
DetachRNG has 1 step DelDevice
RemoveRNG has 2 steps DelObject and DetachCharDev
AttachShmem has 2 steps AttachCharDev and AddDevice
DetachShmem has 1 step DelDevice
RemoveShmem has 1 step DetachCharDev
I think my concern was more related to the difference where an RNG has
an Object that gets removed, but Shmem doesn't.
ivshmem device doesn't have a QOM object, so no need to add/del a QOM
object.
and you can check this doc:
http://wiki.qemu.org/TexiDemo
and see "Inter-VM Shared Memory device" part
After closer inspection now it seems that this event generation needs
to
stay. The RemoveShmem code is generating the event not waiting on it
which perhaps is where my thoughts were when I first went through the
changes.
yes, virDomainEventDeviceRemovedNewFromObj() is generating a event and
it can trigger the callback functions which registered in qemu driver.
Thanks for your help and reply
Luayo
John
> Thanks for your quick reply !
>
> Luyao
>
>> John
>>
>> [...]
>>
8:52 p.m.
New subject: [libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
On 12/21/2015 10:24 AM, lhuang wrote:
On 12/17/2015 08:26 PM, John Ferlan wrote:
>
> On 12/16/2015 09:43 PM, lhuang wrote:
>>
>> On 12/15/2015 08:39 PM, John Ferlan wrote:
>>> [...]
>>>
>>>>>> + if (rc < 0)
>>>>>> + return -1;
>>>>>> +
>>>>> I know this is a copy of the RemoveRNGDevice; however, this code
>>>>> doesn't
>>>>> remove an 'obj'. In fact, if !shmem->server.enabled, then
we don't
>>>>> enter
>>>>> the monitor at all.
>>>>>
>>>>> Thus the following event probably won't happen...
>>>> I am not sure what your mean is ... i guess your mean the device
>>>> remove
>>>> event we get from qmp monitor won't happen ? we will get that
>>>> event if
>>>> qemu remove shmem device success, it should always happen if qemu
>>>> really
>>>> remove it and there is no bugs on qemu :)
>>>>
>>> While reviewing I got lazy and didn't check the non hotplug case to
>>> how
>>> shmem is added to the vm, but the point I was trying to make is
>>> that "if
>>> (shmem->server.enabled)" fails (e.g. is false), then there is no
"rc =
>>> qemuMonitorDelObject(priv->mon, objAlias);" call in this API
>>> (similar to
>>> RNG code), thus how does the following event get triggered? Even if
>>> the
>>> condition was true, does detaching the char dev cause the event to be
>>> triggered?
>>> I thought the event was related to the DelObject code, but I didn't go
>>> follow that code
>> Oh, i see, event is not related to the object delete, i guess you have
>> checked the code and know how it works now ;-)
> Maybe you misunderstood - maybe I misunderstood your response... I
> didn't go searching through the code, but something just didn't seem
> right when looking so I wanted to note it.
Looks like i misunderstood :)
> Since I figured your model was RNG my thought process was how does RNG
> do it and what is different...
>
> AttachRNG has 3 steps AttachCharDev, AddObject, and AddDevice
> DetachRNG has 1 step DelDevice
> RemoveRNG has 2 steps DelObject and DetachCharDev
>
>
> AttachShmem has 2 steps AttachCharDev and AddDevice
> DetachShmem has 1 step DelDevice
> RemoveShmem has 1 step DetachCharDev
>
> I think my concern was more related to the difference where an RNG has
> an Object that gets removed, but Shmem doesn't.
ivshmem device doesn't have a QOM object, so no need to add/del a QOM
object.
and you can check this doc:
http://wiki.qemu.org/TexiDemo
and see "Inter-VM Shared Memory device" part
ivshmem device could have a object when it use x-memdev, so seems we
will add DelObject in RemoveShmem in future :) (maybe when someone
implement ivshmem with hugepage support)
Luyao
> After closer inspection now it seems that this event generation
needs to
> stay. The RemoveShmem code is generating the event not waiting on it
> which perhaps is where my thoughts were when I first went through the
> changes.
yes, virDomainEventDeviceRemovedNewFromObj() is generating a event and
it can trigger the callback functions which registered in qemu driver.
Thanks for your help and reply
Luayo
> John
>
>> Thanks for your quick reply !
>>
>> Luyao
>>
>>> John
>>>
>>> [...]
>>>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3279
days inactive
3304
days old
18 comments
3 participants
participants (3)
-
John Ferlan -
lhuang -
Luyao Huang