Re: [PATCH v2] Add page_per_vq flag to the 'driver' element of virtio devices
On Thu, May 06, 2021 at 17:42:58 +0000, Moshe Levi wrote:
> -----Original Message-----
> From: Peter Krempa <pkrempa(a)redhat.com>
> Sent: Thursday, May 6, 2021 8:16 PM
> To: Gavi Teitz <gavi(a)nvidia.com>
> Cc: libvir-list(a)redhat.com; Moshe Levi <moshele(a)nvidia.com>
> Subject: Re: [PATCH v2] Add page_per_vq flag to the 'driver' element of
> virtio devices
>
> External email: Use caution opening links or attachments
>
>
> On Wed, May 05, 2021 at 10:25:06 +0300, Gavi Teitz wrote:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> >
> illa.redhat.com%2Fshow_bug.cgi%3Fid%3D1925363&data=04%7C01%7C
> moshe
> >
> le%40nvidia.com%7C600a9a0f84f042a47ca208d910b2a1c4%7C43083d1572734
> 0c1b
> >
> 7db39efd9ccc17a%7C0%7C0%7C637559181678971352%7CUnknown%7CTWFp
> bGZsb3d8e
> >
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C10
> >
> 00&sdata=790YPhVCCgI1MUt5in9ul0bDhgBrPipXI61DkNChKdw%3D&am
> p;reserv
> > ed=0
> >
> > Add support for setting the page-per-vq flag, which is important for
> > vdpa with vhost-user performance.
> >
> > Signed-off-by: Gavi Teitz <gavi(a)nvidia.com>
> > ---
> > docs/formatdomain.rst | 11 +++++-
> > docs/schemas/domaincommon.rng | 5 +++
> > src/conf/domain_conf.c | 10 +++++
> > src/conf/domain_conf.h | 1 +
> > src/qemu/qemu_command.c | 5 +++
> > src/qemu/qemu_hotplug.c | 1 +
> > .../net-virtio-page-per-vq.x86_64-latest.args | 38
> ++++++++++++++++++
> > tests/qemuxml2argvdata/net-virtio-page-per-vq.xml | 29
> ++++++++++++++
> > tests/qemuxml2argvtest.c | 1 +
> > .../net-virtio-page-per-vq.x86_64-latest.xml | 46
> ++++++++++++++++++++++
> > tests/qemuxml2xmltest.c | 1 +
> > 11 files changed, 147 insertions(+), 1 deletion(-) create mode
> > 100644
> > tests/qemuxml2argvdata/net-virtio-page-per-vq.x86_64-latest.args
> > create mode 100644 tests/qemuxml2argvdata/net-virtio-page-per-vq.xml
> > create mode 100644
> > tests/qemuxml2xmloutdata/net-virtio-page-per-vq.x86_64-latest.xml
>
> Not a full review, but a note for other reviewers; it's still missing the ABI
> stability check.
So we didn't understand what is ABI stability check. Can you point to the relevant
code and explain what need to be added?
I've already noted that in my reply to v1.
virDomainDefCheckABIStabilityFlags compares two virDomainDef objects to
see whether somethign relating guest ABI has changed. The function calls
virDomainNetDefCheckABIStability which checks the ABI of individual
network devices.
In that function we either check things directly or delegate some common
checkers such as virtio options.
Since this is a virtio option you must assess whether it makes sense for
any other virtio device too or it's just network device specific.