[libvirt] [RFC PATCH 0/5] ivshmem support
The following patches are an implementation proposal for the ivshmem device support in libvirt. Any feedback is welcome. Note: SELinux is not supported (need to be done for the next patchset version) Maxime Leroy (5): doc: schema: Add basic documentation for the ivshmem support conf: Parse and format ivshmem device XML qemu: Add cap flag QEMU_CAPS_IVSHMEM qemu: Build command line for ivshmem device tests: Add tests for ivshmem device handling docs/formatdomain.html.in | 72 +++++++ docs/schemas/domaincommon.rng | 57 ++++++ src/conf/domain_conf.c | 234 ++++++++++++++++++++++- src/conf/domain_conf.h | 40 ++++ src/libvirt_private.syms | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 77 ++++++++ src/qemu/qemu_command.h | 4 + src/qemu/qemu_hotplug.c | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemuhelptest.c | 15 +- tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 10 + tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 36 ++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 2 + 21 files changed, 558 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml -- 1.9.3
This patch documents XML elements used for support of ivshmem devices. About ivshmem, please see the following documentation: http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt (Ivshmem documentation for qemu will be updated soon: https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02974.html) In the devices section in the domain XML users may specify: - For ivshmem device using an ivshmem server: <ivshmem use_server='yes' role='master'/> <source file='/tmp/socket-ivshmem0'/> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </ivshmem> Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt. - For ivshmem device not using an ivshmem server: <ivshmem use_server='no' role='peer'/> <source file='ivshmem1'/> <size unit='M'>32</size> </ivshmem> Note: the ivshmem shm needs to be created before creating the VM. It's not done by libvirt. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- docs/formatdomain.html.in | 72 +++++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e5b1adb..9a9a6fa 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4035,6 +4035,78 @@ qemu-kvm -net nic,model=? /dev/null <code><model></code> element is mandatory. </p> + <h4><a name="elementsIvshmem">Ivshmem device</a></h4> + + <p> + An ivshmem (i.e. Inter-VM shared memory) device allows to share a memory + region (created on the host via the POSIX shared memory API) between + multiple QEMU processes running different guests. + For more information, please see the + <a href="http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt"> + ivshmem documentation of qemu</a>. + </p> + +<pre> + ... + <devices> + <ivshmem use_server='yes' role='master'/> + <source file='/tmp/socket-ivshmem0'/> + <size unit='M'>32</size> + <msi vectors='32' ioeventfd='on'/> + </ivshmem> + <ivshmem use_server='no' role='peer'/> + <source file='ivshmem1'/> + <size unit='M'>32</size> + </ivshmem> + </devices> + ...</pre> + + <dl> + <dt><code>ivshmem</code></dt> + <dd>The <code>ivshmem</code> element has a mandatory <code>use_server</code> + attribute which takes the value "yes" or "no": + <dl> + <dt><code>"yes"</code></dt> + <dd> + Configure the ivshmen device to connect to the ivshmem server via a unix socket. + </dd> + <dt><code>"no"</code></dt> + <dd> + Configure the ivshmen device with a shared memory file. + </dd> + </dl> + </dd> + <dt><code>role</code></dt> + <dd>The <code>role</code> attribute is optional. It takes the value "master" or "peer": + <dl> + <dt><code>"master"</code></dt> + <dd> + On migration, the guest will copy the shared memory to the destination host. + </dd> + <dt><code>"peer"</code></dt> + <dd> + The device should be detached and then reattached after migration using + the PCI hotplug support. + </dd> + </dl> + Note: PCI hotplug is not supported for ivshmem device in libvirt. + </dd> + <dt><code>size</code></dt> + <dd>The optional <code>size</code> element specifies the size of the shared memory file. + </dd> + <dt><code>source</code></dt> + <dd>The <code>source</code> element is mandatory. + The <code>file</code> attribute specifies the ivshmem server socket file in server mode. + In non server mode, it specifies the shared memory file. + </dd> + <dt><code>msi</code></dt> + <dd>The optional <code>msi</code> element allows to enable MSI interrupts. This option + can only be used in server mode. The <code>vectors</code> attribute + allows to specify the number of interrupt vectors. The <code>ioeventd</code> attribute + allows to enable ioeventfd. + </dd> + </dl> + <h4><a name="elementsInput">Input devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 11f0fd0..8229921 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3220,6 +3220,62 @@ </optional> </element> </define> + + <define name="ivshmem"> + <element name="ivshmem"> + <interleave> + <choice> + <group> + <attribute name="use_server"> + <value>yes</value> + </attribute> + <optional> + <element name="msi"> + <optional> + <ref name="ioeventfd"/> + </optional> + <optional> + <attribute name="vectors"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </element> + </optional> + </group> + <group> + <attribute name="use_server"> + <value>no</value> + </attribute> + </group> + </choice> + <element name="source"> + <attribute name="file"> + <text/> + </attribute> + </element> + <optional> + <attribute name="role"> + <choice> + <value>master</value> + <value>peer</value> + </choice> + </attribute> + </optional> + <optional> + <element name="size"> + <ref name="scaledInteger"/> + </element> + </optional> + <optional> + <ref name="alias"/> + </optional> + <optional> + <ref name="address"/> + </optional> + </interleave> + </element> + </define> + <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3767,6 +3823,7 @@ <ref name="redirfilter"/> <ref name="rng"/> <ref name="tpm"/> + <ref name="ivshmem"/> </choice> </zeroOrMore> <optional> -- 1.9.3
On 2014/8/6 0:48, Maxime Leroy wrote:
This patch documents XML elements used for support of ivshmem devices.
About ivshmem, please see the following documentation: http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt (Ivshmem documentation for qemu will be updated soon: https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02974.html)
In the devices section in the domain XML users may specify:
- For ivshmem device using an ivshmem server:
<ivshmem use_server='yes' role='master'/> <source file='/tmp/socket-ivshmem0'/>
I prefer to use <source mode='connect' path='/tmp/socket-ivshmem0'/> . So when ParesXML and Format functions are needed, we can use virDomainChrSourceDef*(), like vmchannel device. What do you think about it ?
<size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </ivshmem>
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
- For ivshmem device not using an ivshmem server:
<ivshmem use_server='no' role='peer'/> <source file='ivshmem1'/> <size unit='M'>32</size> </ivshmem>
Note: the ivshmem shm needs to be created before creating the VM. It's not done by libvirt.
Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- docs/formatdomain.html.in | 72 +++++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e5b1adb..9a9a6fa 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4035,6 +4035,78 @@ qemu-kvm -net nic,model=? /dev/null <code><model></code> element is mandatory. </p>
+ <h4><a name="elementsIvshmem">Ivshmem device</a></h4> + + <p> + An ivshmem (i.e. Inter-VM shared memory) device allows to share a memory + region (created on the host via the POSIX shared memory API) between + multiple QEMU processes running different guests. + For more information, please see the + <a href="http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt"> + ivshmem documentation of qemu</a>. + </p> + +<pre> + ... + <devices> + <ivshmem use_server='yes' role='master'/> + <source file='/tmp/socket-ivshmem0'/> + <size unit='M'>32</size> + <msi vectors='32' ioeventfd='on'/> + </ivshmem> + <ivshmem use_server='no' role='peer'/> + <source file='ivshmem1'/> + <size unit='M'>32</size> + </ivshmem> + </devices> + ...</pre> + + <dl> + <dt><code>ivshmem</code></dt> + <dd>The <code>ivshmem</code> element has a mandatory <code>use_server</code> + attribute which takes the value "yes" or "no": + <dl> + <dt><code>"yes"</code></dt> + <dd> + Configure the ivshmen device to connect to the ivshmem server via a unix socket. + </dd> + <dt><code>"no"</code></dt> + <dd> + Configure the ivshmen device with a shared memory file. + </dd> + </dl> + </dd> + <dt><code>role</code></dt> + <dd>The <code>role</code> attribute is optional. It takes the value "master" or "peer": + <dl> + <dt><code>"master"</code></dt> + <dd> + On migration, the guest will copy the shared memory to the destination host. + </dd> + <dt><code>"peer"</code></dt> + <dd> + The device should be detached and then reattached after migration using + the PCI hotplug support. + </dd> + </dl> + Note: PCI hotplug is not supported for ivshmem device in libvirt. + </dd> + <dt><code>size</code></dt> + <dd>The optional <code>size</code> element specifies the size of the shared memory file. + </dd> + <dt><code>source</code></dt> + <dd>The <code>source</code> element is mandatory. + The <code>file</code> attribute specifies the ivshmem server socket file in server mode. + In non server mode, it specifies the shared memory file. + </dd> + <dt><code>msi</code></dt> + <dd>The optional <code>msi</code> element allows to enable MSI interrupts. This option + can only be used in server mode. The <code>vectors</code> attribute + allows to specify the number of interrupt vectors. The <code>ioeventd</code> attribute + allows to enable ioeventfd. + </dd> + </dl> + <h4><a name="elementsInput">Input devices</a></h4>
<p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 11f0fd0..8229921 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3220,6 +3220,62 @@ </optional> </element> </define> + + <define name="ivshmem"> + <element name="ivshmem"> + <interleave> + <choice> + <group> + <attribute name="use_server"> + <value>yes</value> + </attribute> + <optional> + <element name="msi"> + <optional> + <ref name="ioeventfd"/> + </optional> + <optional> + <attribute name="vectors"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </element> + </optional> + </group> + <group> + <attribute name="use_server"> + <value>no</value> + </attribute> + </group> + </choice> + <element name="source"> + <attribute name="file"> + <text/> + </attribute> + </element> + <optional> + <attribute name="role"> + <choice> + <value>master</value> + <value>peer</value> + </choice> + </attribute> + </optional> + <optional> + <element name="size"> + <ref name="scaledInteger"/> + </element> + </optional> + <optional> + <ref name="alias"/> + </optional> + <optional> + <ref name="address"/> + </optional> + </interleave> + </element> + </define> + <define name="memballoon"> <element name="memballoon"> <attribute name="model"> @@ -3767,6 +3823,7 @@ <ref name="redirfilter"/> <ref name="rng"/> <ref name="tpm"/> + <ref name="ivshmem"/> </choice> </zeroOrMore> <optional>
On Wed, Aug 6, 2014 at 11:41 AM, Wang Rui <moon.wangrui@huawei.com> wrote:
On 2014/8/6 0:48, Maxime Leroy wrote:
This patch documents XML elements used for support of ivshmem devices.
About ivshmem, please see the following documentation: http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt (Ivshmem documentation for qemu will be updated soon: https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02974.html)
In the devices section in the domain XML users may specify:
- For ivshmem device using an ivshmem server:
<ivshmem use_server='yes' role='master'/> <source file='/tmp/socket-ivshmem0'/>
I prefer to use <source mode='connect' path='/tmp/socket-ivshmem0'/> . So when ParesXML and Format functions are needed, we can use virDomainChrSourceDef*(), like vmchannel device. What do you think about it ?
First, thanks for the review. I was thinking of using virDomainChrSourceDef to improve this patch. So the format of the xml needs to be updated accordingly: - For ivshmem device using an ivshmem server: <ivshmem type='unix'> <source mode='connect' path='/tmp/socket-ivshmem0'/> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </ivshmem> - For ivshmem device using directly a shared memory <ivshmem type='file'> <source path='ivshmem0'> <size unit='M'>32</size> </ivshmem> Thus, 'use_server' attribute will be replaced by 'type' attribute (unix or file). Are you ok with that ?
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
This patch documents XML elements used for support of ivshmem devices.
At first, I'd like to thank you for the proposal. There were numerous requests for this, but since ivshmem is not known much, it's not easy to get it in. I'll get to the details later.
About ivshmem, please see the following documentation: http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt (Ivshmem documentation for qemu will be updated soon: https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02974.html)
The documentation is partial and confusing. I hope that will change, but there might be more changes than that. The code should be re-factored, command-line options changed, etc.
In the devices section in the domain XML users may specify:
- For ivshmem device using an ivshmem server:
<ivshmem use_server='yes' role='master'/> <source file='/tmp/socket-ivshmem0'/> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </ivshmem>
At first, we should be more generic about the name if we want other hypervisors to benefit from it. The idea is that ivshmem is not something new that nobody else than qemu can do. It's just a shared memory, every other hypervisor out there can come up with something similar to this, so we don't want this to be called ivshmem. One more reason for that is that it doesn't just share memory between VMs, but between host <-> guest too. Something like a 'shmem' should be fine, I guess. I prolonged the paragraph to stress out this is not qemu-only feature (or might not be in the future) and we should be prepared for that. Because of that, we should be more generic about more things than just the name. Another thing that bothers me (and bothered me with earlier ivshmem-related proposals as well) is that role='(master|peer)' thing. That thing does not mean squat for qemu and should be removed completely. Looking at the code the only thing that role=peer (non-default, by the way) does is that it disables migration... That's it. That's another sign of the ivshmem code maturity inside QEMU that we should keep in mind when designing the XML schema.
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
- For ivshmem device not using an ivshmem server:
<ivshmem use_server='no' role='peer'/> <source file='ivshmem1'/> <size unit='M'>32</size> </ivshmem>
Note: the ivshmem shm needs to be created before creating the VM. It's not done by libvirt.
Isn't it done by qemu automatically? If it's not, the same as for the ivshmem server applies. I had one idea to deal with most of the problems (but adding a lot of code); let me outline that. From the global POV, we want something that will fully work remotely, we want to be able to start it, stop it, assign it to domains, etc. We might even migrate it in the future, but that's another storyline. It must be generic enough, as it can change a lot in the future. And so on. One way out of this mess is to have yet another driver, let's call it shmem driver. That driver would have APIs to define, undefine, ..., start and stop shared memory regions. Those would have their own XML, and domain XML would only have a device <shmem name='asdf_mem'/> or <shmem uuid='...'/> that would mean the domain will be started with a shared memory region defined in the shmem driver. That way it could be shared even between hypervisors. At first it seemed like an unreasonable solution, but the more I think about it the more I like it. It's not a requirement to get your patches it, though, I just wanted to share this in case you (or anyone else) find it compelling enough to try it. Martin
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
This patch documents XML elements used for support of ivshmem devices.
At first, I'd like to thank you for the proposal. There were numerous requests for this, but since ivshmem is not known much, it's not easy to get it in. I'll get to the details later.
You welcome. ;) Thanks for the review. [...]
At first, we should be more generic about the name if we want other hypervisors to benefit from it. The idea is that ivshmem is not something new that nobody else than qemu can do. It's just a shared memory, every other hypervisor out there can come up with something similar to this, so we don't want this to be called ivshmem. One more reason for that is that it doesn't just share memory between VMs, but between host <-> guest too. Something like a 'shmem' should be fine, I guess.
I agree with you, shmem is a better name for ivshmem. In such a case, should ivshmem be renamed in QEMU ? This would break compatibility with older versions of QEMU. If we change ivshmem name in QEMU, we need to manage this case in libvirt. So, I am not sure about this point.
I prolonged the paragraph to stress out this is not qemu-only feature (or might not be in the future) and we should be prepared for that. Because of that, we should be more generic about more things than just the name.
Another thing that bothers me (and bothered me with earlier ivshmem-related proposals as well) is that role='(master|peer)' thing. That thing does not mean squat for qemu and should be removed completely. Looking at the code the only thing that role=peer (non-default, by the way) does is that it disables migration... That's it. That's another sign of the ivshmem code maturity inside QEMU that we should keep in mind when designing the XML schema.
Ok. I added this role to be coherent with the initial proposals. I agree with you, we should wait that live migration is properly supported in ivshmem QEMU before adding any options related to that in libvirt. So I will remove this role to avoid adding confusions.
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not. I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server. Are you ok with that ?
- For ivshmem device not using an ivshmem server:
<ivshmem use_server='no' role='peer'/> <source file='ivshmem1'/> <size unit='M'>32</size> </ivshmem>
Note: the ivshmem shm needs to be created before creating the VM. It's not done by libvirt.
Isn't it done by qemu automatically? If it's not, the same as for the ivshmem server applies.
Just checked the QEMU code, QEMU creates the file if it doesn't exist yet. So my mistake ;)
I had one idea to deal with most of the problems (but adding a lot of code); let me outline that. From the global POV, we want something that will fully work remotely, we want to be able to start it, stop it, assign it to domains, etc. We might even migrate it in the future, but that's another storyline. It must be generic enough, as it can change a lot in the future. And so on. One way out of this mess is to have yet another driver, let's call it shmem driver. That driver would have APIs to define, undefine, ..., start and stop shared memory regions. Those would have their own XML, and domain XML would only have a device <shmem name='asdf_mem'/> or <shmem uuid='...'/> that would mean the domain will be started with a shared memory region defined in the shmem driver. That way it could be shared even between hypervisors.
It seems a nice idea to have a more generic way to share memory between guests or host. When can you share a spec or a RFC patch about it?
At first it seemed like an unreasonable solution, but the more I think about it the more I like it. It's not a requirement to get your patches it, though, I just wanted to share this in case you (or anyone else) find it compelling enough to try it.
I will keep focusing about ivshmem support as QEMU is supporting it for now. I will be on holiday next week. Based on the the comments, I will send a new version after my holidays. Maxime
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
This patch documents XML elements used for support of ivshmem devices.
At first, I'd like to thank you for the proposal. There were numerous requests for this, but since ivshmem is not known much, it's not easy to get it in. I'll get to the details later.
You welcome. ;) Thanks for the review.
[...]
At first, we should be more generic about the name if we want other hypervisors to benefit from it. The idea is that ivshmem is not something new that nobody else than qemu can do. It's just a shared memory, every other hypervisor out there can come up with something similar to this, so we don't want this to be called ivshmem. One more reason for that is that it doesn't just share memory between VMs, but between host <-> guest too. Something like a 'shmem' should be fine, I guess.
I agree with you, shmem is a better name for ivshmem.
In such a case, should ivshmem be renamed in QEMU ?
This would break compatibility with older versions of QEMU.
If we change ivshmem name in QEMU, we need to manage this case in libvirt.
So, I am not sure about this point.
No need to change anything in QEMU. That name is because libvirt supports more than just QEMU. For example if we add support for that in lxc/xen/bhyve/whatever, it might not be called ivshmem and we should be generic.
I prolonged the paragraph to stress out this is not qemu-only feature (or might not be in the future) and we should be prepared for that. Because of that, we should be more generic about more things than just the name.
Another thing that bothers me (and bothered me with earlier ivshmem-related proposals as well) is that role='(master|peer)' thing. That thing does not mean squat for qemu and should be removed completely. Looking at the code the only thing that role=peer (non-default, by the way) does is that it disables migration... That's it. That's another sign of the ivshmem code maturity inside QEMU that we should keep in mind when designing the XML schema.
Ok. I added this role to be coherent with the initial proposals.
I agree with you, we should wait that live migration is properly supported in ivshmem QEMU before adding any options related to that in libvirt.
It is "supported" in a sense that it will migrate. But that doesn't mean it will work. The data on destination host might not be there and there might even be different data. The guest must count with that, but that's another issue.
So I will remove this role to avoid adding confusions.
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
There was a suggestion of implementing the non-server variant first and expand it to the variant with server afterwards. That might be the best solution because we'll have bit more time to see the re-factoring differences in QEMU as well. And we can concentrate on other details.
- For ivshmem device not using an ivshmem server:
<ivshmem use_server='no' role='peer'/> <source file='ivshmem1'/> <size unit='M'>32</size> </ivshmem>
Note: the ivshmem shm needs to be created before creating the VM. It's not done by libvirt.
Isn't it done by qemu automatically? If it's not, the same as for the ivshmem server applies.
Just checked the QEMU code, QEMU creates the file if it doesn't exist yet. So my mistake ;)
I had one idea to deal with most of the problems (but adding a lot of code); let me outline that. From the global POV, we want something that will fully work remotely, we want to be able to start it, stop it, assign it to domains, etc. We might even migrate it in the future, but that's another storyline. It must be generic enough, as it can change a lot in the future. And so on. One way out of this mess is to have yet another driver, let's call it shmem driver. That driver would have APIs to define, undefine, ..., start and stop shared memory regions. Those would have their own XML, and domain XML would only have a device <shmem name='asdf_mem'/> or <shmem uuid='...'/> that would mean the domain will be started with a shared memory region defined in the shmem driver. That way it could be shared even between hypervisors.
It seems a nice idea to have a more generic way to share memory between guests or host. When can you share a spec or a RFC patch about it?
I wasn't going to and I won't be in near future. I'm also missing someone else's (from the maiainers) opinion, so I can't even know if this makes sense or if it's totally exaggerated. If someone is willing to code this in, I'll gladly review it, though.
At first it seemed like an unreasonable solution, but the more I think about it the more I like it. It's not a requirement to get your patches it, though, I just wanted to share this in case you (or anyone else) find it compelling enough to try it.
I will keep focusing about ivshmem support as QEMU is supporting it for now. I will be on holiday next week. Based on the the comments, I will send a new version after my holidays.
Maxime
On Fri, Aug 8, 2014 at 11:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
[...]
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
There was a suggestion of implementing the non-server variant first and expand it to the variant with server afterwards. That might be the best solution because we'll have bit more time to see the re-factoring differences in QEMU as well. And we can concentrate on other details.
I would prefer to have ivshmen server and non-server mode supported in libvirt with these patches; because the XML format need to be designed with both at the same time. The new XML format supporting a start or not of ivshmem server could be: <shmem type='ivshmem'> <shm file='ivshmem0'> <server socket='/tmp/socket-ivshmem0' start='yes'> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </shmem> Note: This new XML format can support different types of shmem. After my holiday, I am going to check how to implement this feature. What do you think about this XML format? Any hints to develop this feature (i.e. starting ivshmen server in libvirt) is welcomed. I assume I need to add a new file: src/util/virivshmemserver.c to add a new function virIvshmemServerRun() and to use it in qemu_command.c. How can I check whether an ivshmem-server application is installed or not on the host ? Are there other equivalent behaviors into libvirt? Maxime
On Fri, Aug 08, 2014 at 02:07:56PM +0200, Maxime Leroy wrote:
On Fri, Aug 8, 2014 at 11:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
[...]
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
There was a suggestion of implementing the non-server variant first and expand it to the variant with server afterwards. That might be the best solution because we'll have bit more time to see the re-factoring differences in QEMU as well. And we can concentrate on other details.
I would prefer to have ivshmen server and non-server mode supported in libvirt with these patches; because the XML format need to be designed with both at the same time.
The new XML format supporting a start or not of ivshmem server could be:
<shmem type='ivshmem'> <shm file='ivshmem0'> <server socket='/tmp/socket-ivshmem0' start='yes'> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </shmem>
Note: This new XML format can support different types of shmem.
After my holiday, I am going to check how to implement this feature. What do you think about this XML format?
It's better, but I would like this: <shmem name='my_ivshmem_001'> <server socket='/tmp/ivsh-server' start='yes'/> ... </shmem> That could be simplified into (in case of no server): <shmem name='my_ivshmem_001'/> Of course this will have a PCI address afterwards. This design is generic enough that it doesn't mention "ivshmem" (so it can be implemented differently in any hypervisor). What's your opinion on that?
Any hints to develop this feature (i.e. starting ivshmen server in libvirt) is welcomed.
With starting the server in libvirt we'll have to wait until it's in qemu *and* we have to deal with how and when to stop the server (if we are running it).
I assume I need to add a new file: src/util/virivshmemserver.c to add a new function virIvshmemServerRun() and to use it in qemu_command.c.
How can I check whether an ivshmem-server application is installed or not on the host ? Are there other equivalent behaviors into libvirt?
That should be checked by virFileIsExecutable() on a IVSHMEM_SERVER_PATH that would be set by configure.ac (take a look at EBTABLES_PATH in src/util/virfirewall.c for example). I'll also see if I'll be able to get any info from the QEMU side about what can we expect to happen with ivshmem in near future. Martin
(Imported thread from archives, hopefully I won't break threading. If I did, I apologize.) On Aug 08 2014, Martin Kletzander wrote:
On Fri, Aug 08, 2014 at 02:07:56PM +0200, Maxime Leroy wrote:
On Fri, Aug 8, 2014 at 11:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote:
[...]
Note: the ivshmem server needs to be launched before creating the VM. It's not done by libvirt.
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
There was a suggestion of implementing the non-server variant first and expand it to the variant with server afterwards. That might be the best solution because we'll have bit more time to see the re-factoring differences in QEMU as well. And we can concentrate on other details.
I would prefer to have ivshmen server and non-server mode supported in libvirt with these patches; because the XML format need to be designed with both at the same time.
The new XML format supporting a start or not of ivshmem server could be:
<shmem type='ivshmem'> <shm file='ivshmem0'> <server socket='/tmp/socket-ivshmem0' start='yes'> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </shmem>
Note: This new XML format can support different types of shmem.
After my holiday, I am going to check how to implement this feature. What do you think about this XML format?
It's better, but I would like this:
<shmem name='my_ivshmem_001'> <server socket='/tmp/ivsh-server' start='yes'/> ... </shmem>
Keep in mind that libvirt should not allow having both the attribute name and the server in the same shmem element. QEMU currently only issues a warning, but I plan to change that, since it makes no sense to continue when there are two possible sources for the SHMEM's file descriptor.
That could be simplified into (in case of no server):
<shmem name='my_ivshmem_001'/>
Of course this will have a PCI address afterwards.
This design is generic enough that it doesn't mention "ivshmem" (so it can be implemented differently in any hypervisor).
What's your opinion on that?
Any hints to develop this feature (i.e. starting ivshmen server in libvirt) is welcomed.
With starting the server in libvirt we'll have to wait until it's in qemu *and* we have to deal with how and when to stop the server (if we are running it).
I assume I need to add a new file: src/util/virivshmemserver.c to add a new function virIvshmemServerRun() and to use it in qemu_command.c.
How can I check whether an ivshmem-server application is installed or not on the host ? Are there other equivalent behaviors into libvirt?
That should be checked by virFileIsExecutable() on a IVSHMEM_SERVER_PATH that would be set by configure.ac (take a look at EBTABLES_PATH in src/util/virfirewall.c for example).
I'll also see if I'll be able to get any info from the QEMU side about what can we expect to happen with ivshmem in near future.
I plan to make a few mostly internal modifications on how it accesses memory and how the registers are laid out in BAR1. I don't really plan on changing the command line for now, one of my minimal changes were changing 'use64=<int>' to 'use64=<bool>', however MST didn't really like it claiming that it will break existing scripts, so I postponed the patch until I finish off doing the more serious internal stuff. Watch out for patchdump with RFCs on qemu-devel in the next few days... Thanks! Levente Kurusa
On Mon, Aug 11, 2014 at 03:58:27PM +0200, Levente Kurusa wrote:
(Imported thread from archives, hopefully I won't break threading. If I did, I apologize.)
On Aug 08 2014, Martin Kletzander wrote:
On Fri, Aug 08, 2014 at 02:07:56PM +0200, Maxime Leroy wrote:
On Fri, Aug 8, 2014 at 11:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote: >
[...]
>Note: the ivshmem server needs to be launched before > creating the VM. It's not done by libvirt. >
This is a good temporary workaround, but keep in mind that libvirt works remotely as well and for remote machines libvirt should be able to do everything for you to be able to run the machine if necessary. So even if it might not start the server now, it should in the future. That should be at least differentiable by some parameter (maybe you do it somewhere in the code somehow, I haven't got into that).
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
There was a suggestion of implementing the non-server variant first and expand it to the variant with server afterwards. That might be the best solution because we'll have bit more time to see the re-factoring differences in QEMU as well. And we can concentrate on other details.
I would prefer to have ivshmen server and non-server mode supported in libvirt with these patches; because the XML format need to be designed with both at the same time.
The new XML format supporting a start or not of ivshmem server could be:
<shmem type='ivshmem'> <shm file='ivshmem0'> <server socket='/tmp/socket-ivshmem0' start='yes'> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </shmem>
Note: This new XML format can support different types of shmem.
After my holiday, I am going to check how to implement this feature. What do you think about this XML format?
It's better, but I would like this:
<shmem name='my_ivshmem_001'> <server socket='/tmp/ivsh-server' start='yes'/> ... </shmem>
Keep in mind that libvirt should not allow having both the attribute name and the server in the same shmem element. QEMU currently only issues a warning, but I plan to change that, since it makes no sense to continue when there are two possible sources for the SHMEM's file descriptor.
I may have misunderstood, so please let me ask for a clarification. One server can serve only one shared memory segment? So if there are multiple ones, multiple servers must be spawned as well? If one server can handle multiple segments, then how does qemu tell it about which one it's communicating (sending an interrupt for example)? And how does qemu access the memory if it doesn't know the shmem name? I can only think of passing an FD with the new memfd_create() syscall, but that's even in linux-next for around 5 days only, so I don't think that's it. I must say, I was just lazy to read the code, so that's why I have so many questions, sorry for that :)
That could be simplified into (in case of no server):
<shmem name='my_ivshmem_001'/>
Of course this will have a PCI address afterwards.
This design is generic enough that it doesn't mention "ivshmem" (so it can be implemented differently in any hypervisor).
What's your opinion on that?
Any hints to develop this feature (i.e. starting ivshmen server in libvirt) is welcomed.
With starting the server in libvirt we'll have to wait until it's in qemu *and* we have to deal with how and when to stop the server (if we are running it).
I assume I need to add a new file: src/util/virivshmemserver.c to add a new function virIvshmemServerRun() and to use it in qemu_command.c.
How can I check whether an ivshmem-server application is installed or not on the host ? Are there other equivalent behaviors into libvirt?
That should be checked by virFileIsExecutable() on a IVSHMEM_SERVER_PATH that would be set by configure.ac (take a look at EBTABLES_PATH in src/util/virfirewall.c for example).
I'll also see if I'll be able to get any info from the QEMU side about what can we expect to happen with ivshmem in near future.
I plan to make a few mostly internal modifications on how it accesses memory and how the registers are laid out in BAR1.
I don't really plan on changing the command line for now, one of my minimal changes were changing 'use64=<int>' to 'use64=<bool>', however MST didn't really like it claiming that it will break existing scripts, so I postponed the patch until I finish off doing the more serious internal stuff.
Watch out for patchdump with RFCs on qemu-devel in the next few days...
Thanks! Levente Kurusa
Thanks for the info! Martin
On Wednesday, 13 August, 2014 10:42:18 AM, Martin Kletzander wrote:
On Mon, Aug 11, 2014 at 03:58:27PM +0200, Levente Kurusa wrote:
(Imported thread from archives, hopefully I won't break threading. If I did, I apologize.)
On Aug 08 2014, Martin Kletzander wrote:
On Fri, Aug 08, 2014 at 02:07:56PM +0200, Maxime Leroy wrote:
On Fri, Aug 8, 2014 at 11:21 AM, Martin Kletzander <mkletzan@redhat.com> wrote:
On Thu, Aug 07, 2014 at 05:34:35PM +0200, Maxime Leroy wrote:
On Thu, Aug 7, 2014 at 12:33 PM, Martin Kletzander <mkletzan@redhat.com> wrote: > >On Tue, Aug 05, 2014 at 06:48:01PM +0200, Maxime Leroy wrote: >>
[...]
> >>Note: the ivshmem server needs to be launched before >> creating the VM. It's not done by libvirt. >> > >This is a good temporary workaround, but keep in mind that libvirt >works remotely as well and for remote machines libvirt should be able >to do everything for you to be able to run the machine if necessary. >So even if it might not start the server now, it should in the future. >That should be at least differentiable by some parameter (maybe you do >it somewhere in the code somehow, I haven't got into that). >
The new version of ivshmem server has not been accepted yet in QEMU. I think it's too early to have an option to launch an ivshmem server or not.
I will prefer to focus on integrating these patches in libvirt first, before adding a new feature to launch an ivhsmem server.
Are you ok with that ?
There was a suggestion of implementing the non-server variant first and expand it to the variant with server afterwards. That might be the best solution because we'll have bit more time to see the re-factoring differences in QEMU as well. And we can concentrate on other details.
I would prefer to have ivshmen server and non-server mode supported in libvirt with these patches; because the XML format need to be designed with both at the same time.
The new XML format supporting a start or not of ivshmem server could be:
<shmem type='ivshmem'> <shm file='ivshmem0'> <server socket='/tmp/socket-ivshmem0' start='yes'> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </shmem>
Note: This new XML format can support different types of shmem.
After my holiday, I am going to check how to implement this feature. What do you think about this XML format?
It's better, but I would like this:
<shmem name='my_ivshmem_001'> <server socket='/tmp/ivsh-server' start='yes'/> ... </shmem>
Keep in mind that libvirt should not allow having both the attribute name and the server in the same shmem element. QEMU currently only issues a warning, but I plan to change that, since it makes no sense to continue when there are two possible sources for the SHMEM's file descriptor.
I may have misunderstood, so please let me ask for a clarification.
Sure! :-) What I meant here is that if you use the name attribute then QEMU will open/create an SHM named like that. If you use the server, then the server will send the fd for the SHM (see below). If you set 'name' and 'server', then QEMU will have two sources for the fd, (it can get it from the server; or it can open the SHM named $name) and it will issue a WARNING and just ignore the 'name'. I guess this is user error, so I will modify QEMU to make sure it errors out in such case.
One server can serve only one shared memory segment? So if there are multiple ones, multiple servers must be spawned as well? If one server can handle multiple segments, then how does qemu tell it about which one it's communicating (sending an interrupt for example)? And how does qemu access the memory if it doesn't know the shmem name? I can only think of passing an FD with the new memfd_create() syscall, but that's even in linux-next for around 5 days only, so I don't think that's it.
No, one server can only handle one SHM. Currently, the server has to be on the same host as the guests are in order to be able to pass the fd via SCM_RIGHTS (see: cmsg(3)) on a UNIX socket.
I must say, I was just lazy to read the code, so that's why I have so many questions, sorry for that :)
That could be simplified into (in case of no server):
<shmem name='my_ivshmem_001'/>
Of course this will have a PCI address afterwards.
This design is generic enough that it doesn't mention "ivshmem" (so it can be implemented differently in any hypervisor).
What's your opinion on that?
Any hints to develop this feature (i.e. starting ivshmen server in libvirt) is welcomed.
With starting the server in libvirt we'll have to wait until it's in qemu *and* we have to deal with how and when to stop the server (if we are running it).
I assume I need to add a new file: src/util/virivshmemserver.c to add a new function virIvshmemServerRun() and to use it in qemu_command.c.
How can I check whether an ivshmem-server application is installed or not on the host ? Are there other equivalent behaviors into libvirt?
That should be checked by virFileIsExecutable() on a IVSHMEM_SERVER_PATH that would be set by configure.ac (take a look at EBTABLES_PATH in src/util/virfirewall.c for example).
I'll also see if I'll be able to get any info from the QEMU side about what can we expect to happen with ivshmem in near future.
I plan to make a few mostly internal modifications on how it accesses memory and how the registers are laid out in BAR1.
I don't really plan on changing the command line for now, one of my minimal changes were changing 'use64=<int>' to 'use64=<bool>', however MST didn't really like it claiming that it will break existing scripts, so I postponed the patch until I finish off doing the more serious internal stuff.
Watch out for patchdump with RFCs on qemu-devel in the next few days...
Thanks! Levente Kurusa
Thanks for the info! Martin
Cheers, Levente Kurusa
This patch adds configuration support for the ivshmem device as described in the schema in the previous patch. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- src/conf/domain_conf.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 40 ++++++++ src/libvirt_private.syms | 4 + 3 files changed, 277 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c25c74b..829f1bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -234,7 +234,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "chr", "memballoon", "nvram", - "rng") + "rng", + "ivshmem") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", @@ -759,6 +760,15 @@ VIR_ENUM_DECL(virDomainBlockJob) VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST, "", "", "copy", "", "active-commit") +VIR_ENUM_IMPL(virDomainIvshmemServer, VIR_DOMAIN_IVSHMEM_SERVER_LAST, + "yes", + "no"); + +VIR_ENUM_IMPL(virDomainIvshmemRole, VIR_DOMAIN_IVSHMEM_ROLE_LAST, + "default", + "master", + "peer"); + #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE @@ -1686,6 +1696,17 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) VIR_FREE(def); } +void virDomainIvshmemDefFree(virDomainIvshmemDefPtr def) +{ + if (!def) + return; + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def->file); + VIR_FREE(def); +} + void virDomainVideoDefFree(virDomainVideoDefPtr def) { if (!def) @@ -1887,6 +1908,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_NVRAM: virDomainNVRAMDefFree(def->data.nvram); break; + case VIR_DOMAIN_DEVICE_IVSHMEM: + virDomainIvshmemDefFree(def->data.ivshmem); + break; case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_NONE: break; @@ -2128,6 +2152,10 @@ void virDomainDefFree(virDomainDefPtr def) virDomainRedirFilterDefFree(def->redirfilter); + for (i = 0; i < def->nivshmems; i++) + virDomainIvshmemDefFree(def->ivshmems[i]); + VIR_FREE(def->ivshmems); + if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData); @@ -2562,6 +2590,8 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device) return &device->data.memballoon->info; case VIR_DOMAIN_DEVICE_NVRAM: return &device->data.nvram->info; + case VIR_DOMAIN_DEVICE_IVSHMEM: + return &device->data.ivshmem->info; case VIR_DOMAIN_DEVICE_RNG: return &device->data.rng->info; @@ -2777,6 +2807,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->hubs[i]->info, opaque) < 0) return -1; } + device.type = VIR_DOMAIN_DEVICE_IVSHMEM; + for (i = 0; i < def->nivshmems; i++) { + device.data.ivshmem = def->ivshmems[i]; + if (cb(def, &device, &def->ivshmems[i]->info, opaque) < 0) + return -1; + } /* This switch statement is here to trigger compiler warning when adding * a new device type. When you are adding a new field to the switch you @@ -2805,6 +2841,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_IVSHMEM: break; } @@ -9354,6 +9391,124 @@ virDomainNVRAMDefParseXML(xmlNodePtr node, return NULL; } +static virDomainIvshmemDefPtr +virDomainIvshmemDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + virDomainIvshmemDefPtr def; + char *use_server = NULL; + char *role = NULL; + char *ioeventfd = NULL; + char *vectors = NULL; + xmlNodePtr cur; + xmlNodePtr save = ctxt->node; + + if (VIR_ALLOC(def) < 0) + return NULL; + + use_server = virXMLPropString(node, "use_server"); + if (use_server != NULL) { + if ((def->use_server + = virDomainIvshmemServerTypeFromString(use_server)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown ivshmem use_server tyoe '%s'"), use_server); + goto error; + } + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing use_server type")); + goto error; + } + + role = virXMLPropString(node, "role"); + if (role != NULL) { + if ((int)(def->role = virDomainIvshmemRoleTypeFromString(role)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown ivshmem role type '%s'"), role); + goto error; + } + } else + def->role = VIR_DOMAIN_IVSHMEM_ROLE_DEFAULT; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "source")) { + if (!(def->file = virXMLPropString(cur, "file"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("cannot parse <source> 'file' attribute")); + goto error; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "size")) { + if (virDomainParseScaledValue("./size[1]", ctxt, + &def->size, 1, + ULLONG_MAX, true) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("size parsing failed")); + goto error; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "msi")) { + if (def->use_server != VIR_DOMAIN_IVSHMEM_SERVER_ENABLED) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("msi <element> is only supported for server ivshmem")); + goto error; + } + def->msi.enabled = true; + vectors = virXMLPropString(cur, "vectors"); + ioeventfd = virXMLPropString(cur, "ioeventfd"); + + if (vectors && + virStrToLong_ui(vectors, NULL, 10, &def->msi.vectors) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse ivshmem vectors attribute '%s'"), + vectors); + goto error; + } + if (ioeventfd) { + if ((def->msi.ioeventfd = + virTristateSwitchTypeFromString(ioeventfd)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown ivshmem ioeventfd mode '%s'"), + ioeventfd); + goto error; + } + } + } + } + cur = cur->next; + } + + if (!def->file) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing <source> element in ivshmem device")); + goto error; + } + + /* size should be a power of two */ + if (def->size & (def->size-1)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("ivshmem size should be a power of two")); + goto error; + } + + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) + goto error; + + cleanup: + ctxt->node = save; + VIR_FREE(use_server); + VIR_FREE(role); + VIR_FREE(ioeventfd); + VIR_FREE(vectors); + return def; + + error: + virDomainIvshmemDefFree(def); + def = NULL; + goto cleanup; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -10210,6 +10365,10 @@ virDomainDeviceDefParse(const char *xmlStr, if (!(dev->data.nvram = virDomainNVRAMDefParseXML(node, flags))) goto error; break; + case VIR_DOMAIN_DEVICE_IVSHMEM: + if (!(dev->data.ivshmem = virDomainIvshmemDefParseXML(node, ctxt, flags))) + goto error; + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: break; @@ -13102,6 +13261,25 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); } + /* analysis of the ivshmem devices */ + if ((n = virXPathNodeSet("./devices/ivshmem", ctxt, &nodes)) < 0) { + goto error; + } + if (n && VIR_ALLOC_N(def->ivshmems, n) < 0) + goto error; + + node = ctxt->node; + for (i = 0; i < n; i++) { + virDomainIvshmemDefPtr ivshmem; + ctxt->node = nodes[i]; + ivshmem = virDomainIvshmemDefParseXML(nodes[i], ctxt, flags); + if (!ivshmem) + goto error; + + def->ivshmems[def->nivshmems++] = ivshmem; + } + ctxt->node = node; + VIR_FREE(nodes); /* analysis of the user namespace mapping */ if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) @@ -16701,6 +16879,55 @@ static int virDomainPanicDefFormat(virBufferPtr buf, return 0; } +static int virDomainIvshmemDefFormat(virBufferPtr buf, + virDomainIvshmemDefPtr def, + unsigned int flags) +{ + const char *use_server = virDomainIvshmemServerTypeToString(def->use_server); + const char *role = virDomainIvshmemRoleTypeToString(def->role); + + if (!use_server) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected ivshmem server %d"), def->use_server); + return -1; + } + + if (!role) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected ivshmem role %d"), def->role); + return -1; + } + + virBufferAsprintf(buf, "<ivshmem use_server='%s'", use_server); + if (def->role) + virBufferAsprintf(buf, " role='%s'", role); + virBufferAddLit(buf, ">\n"); + + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<source file='%s'/>\n", def->file); + if (def->size) + virBufferAsprintf(buf, "<size unit='M'>%llu</size>\n", + def->size / (1024 * 1024)); + + if (def->use_server == VIR_DOMAIN_IVSHMEM_SERVER_ENABLED && def->msi.enabled) { + virBufferAddLit(buf, "<msi"); + if (def->msi.vectors) + virBufferAsprintf(buf, " vectors='%u'", def->msi.vectors); + if (def->msi.ioeventfd) + virBufferAsprintf(buf, " ioeventfd='%s'", + virTristateSwitchTypeToString(def->msi.ioeventfd)); + virBufferAddLit(buf, "/>\n"); + } + + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</ivshmem>\n"); + + return 0; +} + static int virDomainRNGDefFormat(virBufferPtr buf, virDomainRNGDefPtr def, @@ -18250,6 +18477,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainPanicDefFormat(buf, def->panic) < 0) goto error; + for (n = 0; n < def->nivshmems; n++) + if (virDomainIvshmemDefFormat(buf, def->ivshmems[n], flags) < 0) + goto error; + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</devices>\n"); @@ -19615,6 +19846,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_IVSHMEM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bffc0a5..af499b4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -136,6 +136,9 @@ typedef virDomainPanicDef *virDomainPanicDefPtr; typedef struct _virDomainChrSourceDef virDomainChrSourceDef; typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; +typedef struct _virDomainIvshmemDef virDomainIvshmemDef; +typedef virDomainIvshmemDef *virDomainIvshmemDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -157,6 +160,7 @@ typedef enum { VIR_DOMAIN_DEVICE_MEMBALLOON, VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG, + VIR_DOMAIN_DEVICE_IVSHMEM, VIR_DOMAIN_DEVICE_LAST } virDomainDeviceType; @@ -184,6 +188,7 @@ struct _virDomainDeviceDef { virDomainMemballoonDefPtr memballoon; virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; + virDomainIvshmemDefPtr ivshmem; } data; }; @@ -598,6 +603,22 @@ typedef enum { VIR_DOMAIN_DISK_DISCARD_LAST } virDomainDiskDiscard; +typedef enum { + VIR_DOMAIN_IVSHMEM_SERVER_ENABLED = 0, + VIR_DOMAIN_IVSHMEM_SERVER_DISABLED, + + VIR_DOMAIN_IVSHMEM_SERVER_LAST, +} virDomainIvshmemServer; + + +typedef enum { + VIR_DOMAIN_IVSHMEM_ROLE_DEFAULT = 0, + VIR_DOMAIN_IVSHMEM_ROLE_MASTER, + VIR_DOMAIN_IVSHMEM_ROLE_PEER, + + VIR_DOMAIN_IVSHMEM_ROLE_LAST, +} virDomainIvshmemRole; + typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; struct _virDomainBlockIoTuneInfo { unsigned long long total_bytes_sec; @@ -1485,6 +1506,19 @@ struct _virDomainNVRAMDef { virDomainDeviceInfo info; }; +struct _virDomainIvshmemDef { + int use_server; /* enum virDomainIvshmemServer */ + int role; /* virDomainIvshmemRole */ + unsigned long long size; + char *file; + struct { + bool enabled; + unsigned vectors; + virTristateSwitch ioeventfd; + } msi; + virDomainDeviceInfo info; +}; + typedef enum { VIR_DOMAIN_SMBIOS_NONE = 0, VIR_DOMAIN_SMBIOS_EMULATE, @@ -2006,6 +2040,9 @@ struct _virDomainDef { size_t nrngs; virDomainRNGDefPtr *rngs; + size_t nivshmems; + virDomainIvshmemDefPtr *ivshmems; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2203,6 +2240,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def); +void virDomainIvshmemDefFree(virDomainIvshmemDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, const virDomainDef *def, @@ -2629,6 +2667,8 @@ VIR_ENUM_DECL(virDomainRNGModel) VIR_ENUM_DECL(virDomainRNGBackend) VIR_ENUM_DECL(virDomainTPMModel) VIR_ENUM_DECL(virDomainTPMBackend) +VIR_ENUM_DECL(virDomainIvshmemServer) +VIR_ENUM_DECL(virDomainIvshmemRole) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..794f3dd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -300,6 +300,10 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; +virDomainIvshmemRoleTypeFromString; +virDomainIvshmemRoleTypeToString; +virDomainIvshmemServerTypeFromString; +virDomainIvshmemServerTypeToString; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -- 1.9.3
On 2014/8/6 0:48, Maxime Leroy wrote:
This patch adds configuration support for the ivshmem device as described in the schema in the previous patch.
Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- src/conf/domain_conf.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 40 ++++++++ src/libvirt_private.syms | 4 + 3 files changed, 277 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c25c74b..829f1bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[...]
/* This switch statement is here to trigger compiler warning when adding * a new device type. When you are adding a new field to the switch you @@ -2805,6 +2841,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_IVSHMEM: break;
The function is good in logic. But I think it's better to keep VIR_DOMAIN_DEVICE_LAST the last case. (Also case VIR_DOMAIN_DEVICE_RNG should be moved ahead of case VIR_DOMAIN_DEVICE_LAST)
}
@@ -9354,6 +9391,124 @@ virDomainNVRAMDefParseXML(xmlNodePtr node, return NULL; }
+static virDomainIvshmemDefPtr +virDomainIvshmemDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + virDomainIvshmemDefPtr def; + char *use_server = NULL; + char *role = NULL; + char *ioeventfd = NULL; + char *vectors = NULL; + xmlNodePtr cur; + xmlNodePtr save = ctxt->node; + + if (VIR_ALLOC(def) < 0) + return NULL; + + use_server = virXMLPropString(node, "use_server"); + if (use_server != NULL) { + if ((def->use_server + = virDomainIvshmemServerTypeFromString(use_server)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown ivshmem use_server tyoe '%s'"), use_server);
s/tyoe/type
+ goto error; + } + } else { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing use_server type")); + goto error; + } +
[...]
@@ -10210,6 +10365,10 @@ virDomainDeviceDefParse(const char *xmlStr, if (!(dev->data.nvram = virDomainNVRAMDefParseXML(node, flags))) goto error; break; + case VIR_DOMAIN_DEVICE_IVSHMEM: + if (!(dev->data.ivshmem = virDomainIvshmemDefParseXML(node, ctxt, flags))) + goto error; + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: break; @@ -13102,6 +13261,25 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); }
+ /* analysis of the ivshmem devices */ + if ((n = virXPathNodeSet("./devices/ivshmem", ctxt, &nodes)) < 0) { + goto error; + } + if (n && VIR_ALLOC_N(def->ivshmems, n) < 0) + goto error; + + node = ctxt->node; + for (i = 0; i < n; i++) { + virDomainIvshmemDefPtr ivshmem; + ctxt->node = nodes[i]; + ivshmem = virDomainIvshmemDefParseXML(nodes[i], ctxt, flags); + if (!ivshmem) + goto error; + + def->ivshmems[def->nivshmems++] = ivshmem; + } + ctxt->node = node; + VIR_FREE(nodes);
Here actions: node = ctxt->node; ctxt->node = nodes[i]; ctxt->node = node; I see other devices' xml parsing function virDomainXXXParseXML (such as virDomainRNGDefParseXML). These actions are in the function virDomainXXXparesXML(). So are the actions here are redundant? Or should be moved into virDomainIvshmemDefParseXML.
/* analysis of the user namespace mapping */ if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) @@ -16701,6 +16879,55 @@ static int virDomainPanicDefFormat(virBufferPtr buf, return 0; }
+static int virDomainIvshmemDefFormat(virBufferPtr buf, + virDomainIvshmemDefPtr def, + unsigned int flags) +{ + const char *use_server = virDomainIvshmemServerTypeToString(def->use_server); + const char *role = virDomainIvshmemRoleTypeToString(def->role); + + if (!use_server) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected ivshmem server %d"), def->use_server); + return -1; + } + + if (!role) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected ivshmem role %d"), def->role); + return -1; + } + + virBufferAsprintf(buf, "<ivshmem use_server='%s'", use_server); + if (def->role) + virBufferAsprintf(buf, " role='%s'", role); + virBufferAddLit(buf, ">\n");
def->role is type of int(ENUM). Do you want to check def->role is zero? In other words, is role='default' illegal ?
+ + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<source file='%s'/>\n", def->file); + if (def->size) + virBufferAsprintf(buf, "<size unit='M'>%llu</size>\n", + def->size / (1024 * 1024)); + + if (def->use_server == VIR_DOMAIN_IVSHMEM_SERVER_ENABLED && def->msi.enabled) { + virBufferAddLit(buf, "<msi"); + if (def->msi.vectors) + virBufferAsprintf(buf, " vectors='%u'", def->msi.vectors); + if (def->msi.ioeventfd) + virBufferAsprintf(buf, " ioeventfd='%s'", + virTristateSwitchTypeToString(def->msi.ioeventfd)); + virBufferAddLit(buf, "/>\n"); + } + + if (virDomainDeviceInfoIsSet(&def->info, flags) && + virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</ivshmem>\n"); + + return 0; +} + static int virDomainRNGDefFormat(virBufferPtr buf, virDomainRNGDefPtr def, @@ -18250,6 +18477,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainPanicDefFormat(buf, def->panic) < 0) goto error;
+ for (n = 0; n < def->nivshmems; n++) + if (virDomainIvshmemDefFormat(buf, def->ivshmems[n], flags) < 0) + goto error; + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</devices>\n");
@@ -19615,6 +19846,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_IVSHMEM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bffc0a5..af499b4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -136,6 +136,9 @@ typedef virDomainPanicDef *virDomainPanicDefPtr; typedef struct _virDomainChrSourceDef virDomainChrSourceDef; typedef virDomainChrSourceDef *virDomainChrSourceDefPtr;
+typedef struct _virDomainIvshmemDef virDomainIvshmemDef; +typedef virDomainIvshmemDef *virDomainIvshmemDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -157,6 +160,7 @@ typedef enum { VIR_DOMAIN_DEVICE_MEMBALLOON, VIR_DOMAIN_DEVICE_NVRAM, VIR_DOMAIN_DEVICE_RNG, + VIR_DOMAIN_DEVICE_IVSHMEM,
VIR_DOMAIN_DEVICE_LAST } virDomainDeviceType; @@ -184,6 +188,7 @@ struct _virDomainDeviceDef { virDomainMemballoonDefPtr memballoon; virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; + virDomainIvshmemDefPtr ivshmem; } data; };
@@ -598,6 +603,22 @@ typedef enum { VIR_DOMAIN_DISK_DISCARD_LAST } virDomainDiskDiscard;
+typedef enum { + VIR_DOMAIN_IVSHMEM_SERVER_ENABLED = 0, + VIR_DOMAIN_IVSHMEM_SERVER_DISABLED, + + VIR_DOMAIN_IVSHMEM_SERVER_LAST, +} virDomainIvshmemServer; + + +typedef enum { + VIR_DOMAIN_IVSHMEM_ROLE_DEFAULT = 0, + VIR_DOMAIN_IVSHMEM_ROLE_MASTER, + VIR_DOMAIN_IVSHMEM_ROLE_PEER, + + VIR_DOMAIN_IVSHMEM_ROLE_LAST, +} virDomainIvshmemRole; + typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; struct _virDomainBlockIoTuneInfo { unsigned long long total_bytes_sec; @@ -1485,6 +1506,19 @@ struct _virDomainNVRAMDef { virDomainDeviceInfo info; };
+struct _virDomainIvshmemDef { + int use_server; /* enum virDomainIvshmemServer */ + int role; /* virDomainIvshmemRole */ + unsigned long long size; + char *file; + struct { + bool enabled; + unsigned vectors; + virTristateSwitch ioeventfd; + } msi; + virDomainDeviceInfo info; +}; + typedef enum { VIR_DOMAIN_SMBIOS_NONE = 0, VIR_DOMAIN_SMBIOS_EMULATE, @@ -2006,6 +2040,9 @@ struct _virDomainDef { size_t nrngs; virDomainRNGDefPtr *rngs;
+ size_t nivshmems; + virDomainIvshmemDefPtr *ivshmems; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2203,6 +2240,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def); +void virDomainIvshmemDefFree(virDomainIvshmemDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, const virDomainDef *def, @@ -2629,6 +2667,8 @@ VIR_ENUM_DECL(virDomainRNGModel) VIR_ENUM_DECL(virDomainRNGBackend) VIR_ENUM_DECL(virDomainTPMModel) VIR_ENUM_DECL(virDomainTPMBackend) +VIR_ENUM_DECL(virDomainIvshmemServer) +VIR_ENUM_DECL(virDomainIvshmemRole) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..794f3dd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -300,6 +300,10 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; +virDomainIvshmemRoleTypeFromString; +virDomainIvshmemRoleTypeToString; +virDomainIvshmemServerTypeFromString; +virDomainIvshmemServerTypeToString; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert;
On Wed, Aug 6, 2014 at 11:30 AM, Wang Rui <moon.wangrui@huawei.com> wrote:
On 2014/8/6 0:48, Maxime Leroy wrote: [..]
@@ -2805,6 +2841,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_IVSHMEM: break;
The function is good in logic. But I think it's better to keep VIR_DOMAIN_DEVICE_LAST the last case. (Also case VIR_DOMAIN_DEVICE_RNG should be moved ahead of case VIR_DOMAIN_DEVICE_LAST)
Good catch ;) I will fix it. [...]
+ /* analysis of the ivshmem devices */ + if ((n = virXPathNodeSet("./devices/ivshmem", ctxt, &nodes)) < 0) { + goto error; + } + if (n && VIR_ALLOC_N(def->ivshmems, n) < 0) + goto error; + + node = ctxt->node; + for (i = 0; i < n; i++) { + virDomainIvshmemDefPtr ivshmem; + ctxt->node = nodes[i]; + ivshmem = virDomainIvshmemDefParseXML(nodes[i], ctxt, flags); + if (!ivshmem) + goto error; + + def->ivshmems[def->nivshmems++] = ivshmem; + } + ctxt->node = node; + VIR_FREE(nodes);
Here actions: node = ctxt->node; ctxt->node = nodes[i]; ctxt->node = node; I see other devices' xml parsing function virDomainXXXParseXML (such as virDomainRNGDefParseXML). These actions are in the function virDomainXXXparesXML(). So are the actions here are redundant? Or should be moved into virDomainIvshmemDefParseXML.
The action is not redundant. The virDomainDefParseXML can parse multi ivshmem devices. It's why we loop here on the different ivshmem nodes. In virDomainIvshmemDefParseXML, we iterate on the different nodes of one ivshmem device (like msi node, size node, source node).
On Tue, Aug 05, 2014 at 06:48:02PM +0200, Maxime Leroy wrote:
This patch adds configuration support for the ivshmem device as described in the schema in the previous patch.
Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- src/conf/domain_conf.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 40 ++++++++ src/libvirt_private.syms | 4 + 3 files changed, 277 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c25c74b..829f1bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -234,7 +234,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "chr", "memballoon", "nvram", - "rng") + "rng", + "ivshmem")
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", @@ -759,6 +760,15 @@ VIR_ENUM_DECL(virDomainBlockJob) VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST, "", "", "copy", "", "active-commit")
+VIR_ENUM_IMPL(virDomainIvshmemServer, VIR_DOMAIN_IVSHMEM_SERVER_LAST, + "yes", + "no"); +
Since commit bb018ce6c85ee17711a0d8c122c6861bb189ce56 we have virTristateBool enym type for this.
+VIR_ENUM_IMPL(virDomainIvshmemRole, VIR_DOMAIN_IVSHMEM_ROLE_LAST, + "default", + "master", + "peer"); +
I know I said that already, but just to be sure, this is not needed as it doesn't make sense in qemu :( Martin
Ivshmem is supported by QEMU since 0.13 release. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemuhelptest.c | 15 ++++++++++----- 9 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f69c4d0..f573cb5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -264,6 +264,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "memory-backend-ram", /* 170 */ "numa", "memory-backend-file", + "ivshmem", ); @@ -1483,6 +1484,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-kbd", QEMU_CAPS_DEVICE_USB_KBD }, { "memory-backend-ram", QEMU_CAPS_OBJECT_MEMORY_RAM }, { "memory-backend-file", QEMU_CAPS_OBJECT_MEMORY_FILE }, + { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e80a377..39079cb 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -212,6 +212,7 @@ typedef enum { QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */ QEMU_CAPS_NUMA = 171, /* newer -numa handling with disjoint cpu ranges */ QEMU_CAPS_OBJECT_MEMORY_FILE = 172, /* -object memory-backend-file */ + QEMU_CAPS_DEVICE_IVSHMEM = 173, /* -device ivshmem */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index ebbfb82..951685e 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -115,4 +115,5 @@ <flag name='enable-fips'/> <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index ab631a2..b26e30e 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -128,4 +128,5 @@ <flag name='kvm-pit-lost-tick-policy'/> <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index e710b60..6d267ac 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -129,4 +129,5 @@ <flag name='kvm-pit-lost-tick-policy'/> <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 36758c8..5751465 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -137,4 +137,5 @@ <flag name='spiceport'/> <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index ca2c236..4b18b68 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -144,4 +144,5 @@ <flag name='usb-kbd'/> <flag name='host-pci-multidomain'/> <flag name='msg-timestamp'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index 4b9f693..4b07df1 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -143,4 +143,5 @@ <flag name='host-pci-multidomain'/> <flag name='msg-timestamp'/> <flag name='numa'/> + <flag name='ivshmem'/> </qemuCaps> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 105a563..5b4cce6 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -518,7 +518,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_DEVICE_USB_STORAGE, - QEMU_CAPS_HOST_PCI_MULTIDOMAIN); + QEMU_CAPS_HOST_PCI_MULTIDOMAIN, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -745,7 +746,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_DEVICE_USB_STORAGE); + QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -842,7 +844,8 @@ mymain(void) QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX, QEMU_CAPS_VNC_SHARE_POLICY, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_DEVICE_USB_STORAGE); + QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-1.2.0", 1002000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -951,7 +954,8 @@ mymain(void) QEMU_CAPS_VNC_SHARE_POLICY, QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_USB_STORAGE_REMOVABLE); + QEMU_CAPS_USB_STORAGE_REMOVABLE, + QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -1065,7 +1069,8 @@ mymain(void) QEMU_CAPS_VNC_SHARE_POLICY, QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_USB_STORAGE_REMOVABLE); + QEMU_CAPS_USB_STORAGE_REMOVABLE, + QEMU_CAPS_DEVICE_IVSHMEM); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.9.3
This patch implements support for the ivshmem device in QEMU. Example from this xml: <ivshmem server='yes'' role='master'/> <source file='/tmp/socket-ivshmem0'/> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </ivshmem> The following QEMU line is built: -device ivshmem,size=32m,vectors=32,chardev=charivshmem0,msi=on, ioeventfd=on,role=master -chardev socket,path=/tmp/socket-ivshmem0,id=charivshmem0 Note: PCI hotpluging has not be implemented. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- src/qemu/qemu_command.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 +++ src/qemu/qemu_hotplug.c | 1 + 3 files changed, 82 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a5ff10a..b434023 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1034,6 +1034,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0) return -1; } + for (i = 0; i < def->nivshmems; i++) { + if (virAsprintf(&def->ivshmems[i]->info.alias, "ivshmem%zu", i) < 0) + return -1; + } for (i = 0; i < def->nsmartcards; i++) { if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%zu", i) < 0) return -1; @@ -5032,6 +5036,53 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def, } char * +qemuBuildIvshmemDevStr(virDomainDefPtr def, + virDomainIvshmemDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ivshmem device is not supported by QEMU")); + goto error; + } + + virBufferAddLit(&buf, "ivshmem"); + if (dev->size) + virBufferAsprintf(&buf, ",size=%llum", dev->size / (1024 * 1024)); + if (dev->role) + virBufferAsprintf(&buf, ",role=%s", + virDomainIvshmemRoleTypeToString(dev->role)); + + if (dev->use_server == VIR_DOMAIN_IVSHMEM_SERVER_DISABLED) + virBufferAsprintf(&buf, ",shm=%s", dev->file); + else { + virBufferAsprintf(&buf, ",chardev=char%s", dev->info.alias); + if (dev->msi.enabled) { + virBufferAddLit(&buf, ",msi=on"); + if (dev->msi.vectors) + virBufferAsprintf(&buf, ",vectors=%u", dev->msi.vectors); + if (dev->msi.ioeventfd) + virBufferAsprintf(&buf, ",ioeventfd=%s", + virTristateSwitchTypeToString(dev->msi.ioeventfd)); + } + } + + if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) + goto error; + + if (virBufferCheckError(&buf) < 0) + goto error; + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + +char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) @@ -9317,6 +9368,32 @@ qemuBuildCommandLine(virConnectPtr conn, } } + for (i = 0; i < def->nivshmems; i++) { + virDomainIvshmemDefPtr ivshmem = def->ivshmems[i]; + char *devstr; + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildIvshmemDevStr(def, ivshmem, qemuCaps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (ivshmem->use_server == VIR_DOMAIN_IVSHMEM_SERVER_ENABLED) { + virDomainChrSourceDef source; + + source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + source.data.nix.path = ivshmem->file; + source.data.nix.listen = false; + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&source, ivshmem->info.alias, + qemuCaps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + if (mlock) { unsigned long long memKB; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index b71e964..f3d301a 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -185,6 +185,10 @@ char * qemuBuildHubDevStr(virDomainDefPtr def, char * qemuBuildRedirdevDevStr(virDomainDefPtr def, virDomainRedirdevDefPtr dev, virQEMUCapsPtr qemuCaps); +char * qemuBuildIvshmemDevStr(virDomainDefPtr def, + virDomainIvshmemDefPtr dev, + virQEMUCapsPtr qemuCaps); + int qemuNetworkIfaceConnect(virDomainDefPtr def, virConnectPtr conn, virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 004b6a4..7be3a04 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2824,6 +2824,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_IVSHMEM: case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("don't know how to remove a %s device"), -- 1.9.3
On 2014/8/6 0:48, Maxime Leroy wrote:
This patch implements support for the ivshmem device in QEMU.
Example from this xml:
<ivshmem server='yes'' role='master'/>
s/'yes''/'yes'
<source file='/tmp/socket-ivshmem0'/> <size unit='M'>32</size> <msi vectors='32' ioeventfd='on'/> </ivshmem>
The following QEMU line is built:
-device ivshmem,size=32m,vectors=32,chardev=charivshmem0,msi=on, ioeventfd=on,role=master -chardev socket,path=/tmp/socket-ivshmem0,id=charivshmem0
Note: PCI hotpluging has not be implemented.
Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- src/qemu/qemu_command.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 +++ src/qemu/qemu_hotplug.c | 1 + 3 files changed, 82 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a5ff10a..b434023 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1034,6 +1034,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0) return -1; } + for (i = 0; i < def->nivshmems; i++) { + if (virAsprintf(&def->ivshmems[i]->info.alias, "ivshmem%zu", i) < 0) + return -1; + } for (i = 0; i < def->nsmartcards; i++) { if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%zu", i) < 0) return -1; @@ -5032,6 +5036,53 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def, }
[...]
+char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) @@ -9317,6 +9368,32 @@ qemuBuildCommandLine(virConnectPtr conn, } }
+ for (i = 0; i < def->nivshmems; i++) { + virDomainIvshmemDefPtr ivshmem = def->ivshmems[i]; + char *devstr; + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildIvshmemDevStr(def, ivshmem, qemuCaps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (ivshmem->use_server == VIR_DOMAIN_IVSHMEM_SERVER_ENABLED) { + virDomainChrSourceDef source; + + source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + source.data.nix.path = ivshmem->file; + source.data.nix.listen = false; + + virCommandAddArg(cmd, "-chardev");
In qemuBuildCommandLine() , "-device" and "-chardev" capabilities are checked before most(not all) of virCommandAddArg for devices. I think that will be nicer.
On Wed, Aug 6, 2014 at 12:04 PM, Wang Rui <moon.wangrui@huawei.com> wrote:
On 2014/8/6 0:48, Maxime Leroy wrote:
This patch implements support for the ivshmem device in QEMU.
Example from this xml:
<ivshmem server='yes'' role='master'/>
s/'yes''/'yes'
[..]
+ virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildIvshmemDevStr(def, ivshmem, qemuCaps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (ivshmem->use_server == VIR_DOMAIN_IVSHMEM_SERVER_ENABLED) { + virDomainChrSourceDef source; + + source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + source.data.nix.path = ivshmem->file; + source.data.nix.listen = false; + + virCommandAddArg(cmd, "-chardev");
In qemuBuildCommandLine() , "-device" and "-chardev" capabilities are checked before most(not all) of virCommandAddArg for devices. I think that will be nicer.
ok, I will update accordingly.
Add XML parsing and qemu command line tests for the ivshmem device support. Signed-off-by: Maxime Leroy <maxime.leroy@6wind.com> --- tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 10 +++++++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 36 ++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ tests/qemuxml2xmltest.c | 2 ++ 4 files changed, 51 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args new file mode 100644 index 0000000..c707ba2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-device ivshmem,chardev=charivshmem0,msi=on,vectors=32,ioeventfd=on \ +-chardev socket,id=charivshmem0,path=/tmp/socket-ivshmem0 \ +-device ivshmem,size=32m,role=master,chardev=charivshmem1,msi=on,vectors=32 \ +-chardev socket,id=charivshmem1,path=/tmp/socket-ivshmem1 \ +-device ivshmem,size=32m,role=peer,shm=ivshmem2,bus=pci.0,addr=0x8 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml new file mode 100644 index 0000000..8214869 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + <ivshmem use_server='yes'> + <source file='/tmp/socket-ivshmem0'/> + <msi vectors='32' ioeventfd='on'/> + </ivshmem> + <ivshmem use_server='yes' role='master'> + <source file='/tmp/socket-ivshmem1'/> + <size unit='M'>32</size> + <msi vectors='32'/> + </ivshmem> + <ivshmem use_server='no' role='peer'> + <source file='ivshmem2'/> + <size unit='M'>32</size> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </ivshmem> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 12ecabc..b2a2bc4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1412,6 +1412,9 @@ mymain(void) DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("ivshmem", QEMU_CAPS_PCIDEVICE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 451dedc..9bb692a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -382,6 +382,8 @@ mymain(void) DO_TEST_DIFFERENT("numatune-memnode"); DO_TEST("numatune-memnode-no-memory"); + DO_TEST("ivshmem"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.9.3
On 2014/8/6 0:48, Maxime Leroy wrote:
The following patches are an implementation proposal for the ivshmem device support in libvirt.
Any feedback is welcome.
Note: SELinux is not supported (need to be done for the next patchset version)
Just some questiones: Is there only one master at most on a host? What will happen if two masters(in one VM or two VMS) on a host?
On Aug 06 2014, Wang Rui wrote:
On 2014/8/6 0:48, Maxime Leroy wrote:
The following patches are an implementation proposal for the ivshmem device support in libvirt.
Any feedback is welcome.
Note: SELinux is not supported (need to be done for the next patchset version)
Just some questiones: Is there only one master at most on a host?
Right now, qemu does not have any restriction on the number of masters on a host.
What will happen if two masters(in one VM or two VMS) on a host?
By default, all ivshmem devices are 'master' unless specified to be 'peer' on the command line. The only difference between 'master' and 'peer' is that 'peer' blocks migration. My guess is that this mode-thing was a planned feature to solve migration problems. According to my understanding the following was meant to be: The guests using the same SHM are virtually grouped together, having one (and only one!) master guest, the rest being peer guests. On migration the guest-group can only be migrated to the same host and all together. The master guest will copy over the SHM. Before this happens the ivshmem devices are detached from the peer guests using PCI/ACPI hotplug. Once the master finishes the copy, the guests are migrated and the peers are reattached with a new fd pointing to the new shm object on the new host. So, to answer your question: My guess is that you can have as much master guests on the host, given that each master has a unique SHM. Thanks! Levente Kurusa
On Mon, Aug 11, 2014 at 8:38 AM, Levente Kurusa <lkurusa@redhat.com> wrote:
On Aug 06 2014, Wang Rui wrote:
On 2014/8/6 0:48, Maxime Leroy wrote:
The following patches are an implementation proposal for the ivshmem device support in libvirt.
Any feedback is welcome.
Note: SELinux is not supported (need to be done for the next patchset version)
Just some questiones: Is there only one master at most on a host?
Right now, qemu does not have any restriction on the number of masters on a host.
What will happen if two masters(in one VM or two VMS) on a host?
By default, all ivshmem devices are 'master' unless specified to be 'peer' on the command line. The only difference between 'master' and 'peer' is that 'peer' blocks migration.
My guess is that this mode-thing was a planned feature to solve migration problems. According to my understanding the following was meant to be:
The guests using the same SHM are virtually grouped together, having one (and only one!) master guest, the rest being peer guests. On migration the guest-group can only be migrated to the same host and all together.
The master guest will copy over the SHM. Before this happens the ivshmem devices are detached from the peer guests using PCI/ACPI hotplug. Once the master finishes the copy, the guests are migrated and the peers are reattached with a new fd pointing to the new shm object on the new host.
So, to answer your question: My guess is that you can have as much master guests on the host, given that each master has a unique SHM.
Hello, One master per SHM is the original intent. The goal was to make migration behaviour explicit for each guest. Cam
Thanks! Levente Kurusa
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (5)
-
Cam Macdonell -
Levente Kurusa -
Martin Kletzander -
Maxime Leroy -
Wang Rui