Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Zhoujian (jay)]

> -Original Message-
> From: Qemu-devel [mailto:qemu-devel-
> bounces+jianjay.zhou=huawei@nongnu.org] On Behalf Of Michael S. Tsirkin
> Sent: Wednesday, February 14, 2018 12:52 AM
> To: Peter Maydell <peter.mayd...@linaro.org>
> Cc: QEMU Developers <qemu-devel@nongnu.org>
> Subject: Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features,
> fixes and cleanups
> 
> On Tue, Feb 13, 2018 at 04:33:14PM +, Peter Maydell wrote:
> > On 12 February 2018 at 09:35, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
> > > This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:
> > >
> > > TEST: tests/device-introspect-test... (pid=19530)
> > >   /aarch64/device/introspect/list: OK
> > >   /aarch64/device/introspect/list-fields:  OK
> > >   /aarch64/device/introspect/none: OK
> > >   /aarch64/device/introspect/abstract: OK
> > >   /aarch64/device/introspect/concrete: **
> > > ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
> > > assertion failed: (type != NULL)
> > > Broken pipe
> > > FAIL
> > > GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
> > > (pid=12686)
> > >   /aarch64/device/introspect/abstract-interfaces:  OK
> > > FAIL: tests/device-introspect-test
> > >
> > > (other archs fail too, aarch64 is just the first one we hit). This
> > > error is often "device A instantiates device B, but device B is
> > > conditionally compiled in and A is always compiled; so test fails
> > > trying to create device A on hosts where device B isn't built" I think.
> >
> > This part is indeed that problem -- the 'virtio-crypto-device' object
> > is built only if CONFIG_LINUX, but 'virtio-crypto-pci' is built if
> > CONFIG_VIRTIO_PCI, so on systems where the latter is true but not the
> > former you get a virtio-crypto-pci device that crashes when instantiated.
> >
> > The fix should be
> > --- a/hw/virtio/Makefile.objs
> > +++ b/hw/virtio/Makefile.objs
> > @@ -8,7 +8,7 @@ obj-y += virtio.o virtio-balloon.o
> >  obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
> >  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
> >  obj-$(CONFIG_LINUX) += virtio-crypto.o
> > -obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
> > +obj-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_PCI)) +=
> > +virtio-crypto-pci.o
> >  endif
> >
> >  common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o
> >
> >
> > thanks
> > -- PMM
> 
> Thanks for your help with this!
> 
> I think the root cause is that one of the patches disables virtio-crypto
> build on non linux which used to be enabled.
> 
> This has been silently done by a patch which was supposed to merely add a
> vhost crypto device, I'm sorry I missed that in the review.

Hi Michael, Peter

Sorry for the late response and the trouble I have made, I just returned
from a vacation.
Thanks for your help and pointing out the root cause, I should explicitly
put the config change of virtio-crypto into a separate patch to make
review much easier.

> 
> I've dropped the crypto vhost patches from the pull request for now.

Will fix the issue in the next version, sorry again for the inconvenience.

Regards,
Jay

> 
> Pushed with the same name - should be fine now.
> 
> --
> MST

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Peter Maydell]

On 13 February 2018 at 16:52, Michael S. Tsirkin  wrote:
> I've dropped the crypto vhost patches from the pull request for now.
>
> Pushed with the same name - should be fine now.

Applied the fixed version, thanks.

-- PMM

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Michael S. Tsirkin]

On Tue, Feb 13, 2018 at 04:33:14PM +, Peter Maydell wrote:
> On 12 February 2018 at 09:35, Peter Maydell  wrote:
> > This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:
> >
> > TEST: tests/device-introspect-test... (pid=19530)
> >   /aarch64/device/introspect/list: OK
> >   /aarch64/device/introspect/list-fields:  OK
> >   /aarch64/device/introspect/none: OK
> >   /aarch64/device/introspect/abstract: OK
> >   /aarch64/device/introspect/concrete: **
> > ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
> > assertion failed: (type != NULL)
> > Broken pipe
> > FAIL
> > GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
> > (pid=12686)
> >   /aarch64/device/introspect/abstract-interfaces:  OK
> > FAIL: tests/device-introspect-test
> >
> > (other archs fail too, aarch64 is just the first one we hit). This
> > error is often "device A instantiates device B, but device B is
> > conditionally compiled in and A is always compiled; so test fails
> > trying to create device A on hosts where device B isn't built" I think.
> 
> This part is indeed that problem -- the 'virtio-crypto-device' object
> is built only if CONFIG_LINUX, but 'virtio-crypto-pci' is built if
> CONFIG_VIRTIO_PCI, so on systems where the latter is true but not the
> former you get a virtio-crypto-pci device that crashes when instantiated.
> 
> The fix should be
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -8,7 +8,7 @@ obj-y += virtio.o virtio-balloon.o
>  obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
>  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
>  obj-$(CONFIG_LINUX) += virtio-crypto.o
> -obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
> +obj-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_PCI)) += virtio-crypto-pci.o
>  endif
> 
>  common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o
> 
> 
> thanks
> -- PMM

Thanks for your help with this!

I think the root cause is that one of the patches disables
virtio-crypto build on non linux which used to be enabled.

This has been silently done by a patch which was supposed to
merely add a vhost crypto device, I'm sorry I missed that
in the review.

I've dropped the crypto vhost patches from the pull request for now.

Pushed with the same name - should be fine now.

-- 
MST

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Peter Maydell]

On 12 February 2018 at 09:35, Peter Maydell  wrote:
> This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:
>
> TEST: tests/device-introspect-test... (pid=19530)
>   /aarch64/device/introspect/list: OK
>   /aarch64/device/introspect/list-fields:  OK
>   /aarch64/device/introspect/none: OK
>   /aarch64/device/introspect/abstract: OK
>   /aarch64/device/introspect/concrete: **
> ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
> assertion failed: (type != NULL)
> Broken pipe
> FAIL
> GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
> (pid=12686)
>   /aarch64/device/introspect/abstract-interfaces:  OK
> FAIL: tests/device-introspect-test
>
> (other archs fail too, aarch64 is just the first one we hit). This
> error is often "device A instantiates device B, but device B is
> conditionally compiled in and A is always compiled; so test fails
> trying to create device A on hosts where device B isn't built" I think.

This part is indeed that problem -- the 'virtio-crypto-device' object
is built only if CONFIG_LINUX, but 'virtio-crypto-pci' is built if
CONFIG_VIRTIO_PCI, so on systems where the latter is true but not the
former you get a virtio-crypto-pci device that crashes when instantiated.

The fix should be
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -8,7 +8,7 @@ obj-y += virtio.o virtio-balloon.o
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
 obj-$(CONFIG_LINUX) += virtio-crypto.o
-obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
+obj-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_PCI)) += virtio-crypto-pci.o
 endif

 common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o


thanks
-- PMM

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Peter Maydell]

On 9 February 2018 at 17:07, Michael S. Tsirkin  wrote:
> On Fri, Feb 09, 2018 at 10:06:42AM +, Peter Maydell wrote:
>> On 8 February 2018 at 19:08, Michael S. Tsirkin  wrote:
>> > The following changes since commit 
>> > 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
>> >
>> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
>> > into staging (2018-02-08 14:31:51 +)
>> >
>> > are available in the git repository at:
>> >
>> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>> >
>> > for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:
>> >
>> >   virtio-balloon: include statistics of disk/file caches (2018-02-08 
>> > 21:06:42 +0200)
>> >
>> > 
>> > virtio,vhost,pci,pc: features, fixes and cleanups
>> >
>> > - a new vhost crypto device
>> > - new stats in virtio balloon
>> > - virtio eventfd rework for boot speedup
>> > - vhost memory rework for boot speedup
>> > - fixes and cleanups all over the place
>> >
>> > Signed-off-by: Michael S. Tsirkin 
>> >
>>
>> Hi. This has some format-string issues:

> Fixed up now, new hash d94c5ae38f25a6e77e07007dcd510b7203a687e8

This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:

TEST: tests/device-introspect-test... (pid=19530)
  /aarch64/device/introspect/list: OK
  /aarch64/device/introspect/list-fields:  OK
  /aarch64/device/introspect/none: OK
  /aarch64/device/introspect/abstract: OK
  /aarch64/device/introspect/concrete: **
ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
assertion failed: (type != NULL)
Broken pipe
FAIL
GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
(pid=12686)
  /aarch64/device/introspect/abstract-interfaces:  OK
FAIL: tests/device-introspect-test

(other archs fail too, aarch64 is just the first one we hit). This
error is often "device A instantiates device B, but device B is
conditionally compiled in and A is always compiled; so test fails
trying to create device A on hosts where device B isn't built" I think.

clang build on x86 fails to compile:
/home/petmay01/linaro/qemu-for-merges/backends/cryptodev-vhost-user.c:86:16:
error: address of array 's->vhost_crypto' will always evaluate to
'true' [-Werror,-Wpointer-bool-conversion]
if (s->vhost_crypto) {
~~  ~~~^~~~

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Gonglei (Arei)]

> -Original Message-
> From: Qemu-devel
> [mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org] On
> Behalf Of Peter Maydell
> Sent: Friday, February 09, 2018 6:07 PM
> To: Michael S. Tsirkin
> Cc: QEMU Developers
> Subject: Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, 
> fixes and
> cleanups
> 
> On 8 February 2018 at 19:08, Michael S. Tsirkin <m...@redhat.com> wrote:
> > The following changes since commit
> 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
> >
> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request'
> into staging (2018-02-08 14:31:51 +)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to
> f4ac9b2e04e8d98854a97bc473353207765aa9e7:
> >
> >   virtio-balloon: include statistics of disk/file caches (2018-02-08 
> > 21:06:42
> +0200)
> >
> > 
> > virtio,vhost,pci,pc: features, fixes and cleanups
> >
> > - a new vhost crypto device
> > - new stats in virtio balloon
> > - virtio eventfd rework for boot speedup
> > - vhost memory rework for boot speedup
> > - fixes and cleanups all over the place
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> >
> 
> Hi. This has some format-string issues:
> 
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_start':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:112:26:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>  error_report("failed to init vhost_crypto for queue %lu", i);
>   ^
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_init':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:205:40:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>  cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
> ^
> 
Using %zu instead of %lu will be correct. Michael, could you pls fix it 
directly?

Very sorry for the inconvenience. :(

Thanks,
-Gonglei

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Michael S. Tsirkin]

On Fri, Feb 09, 2018 at 10:06:42AM +, Peter Maydell wrote:
> On 8 February 2018 at 19:08, Michael S. Tsirkin  wrote:
> > The following changes since commit 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
> >
> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
> > into staging (2018-02-08 14:31:51 +)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:
> >
> >   virtio-balloon: include statistics of disk/file caches (2018-02-08 
> > 21:06:42 +0200)
> >
> > 
> > virtio,vhost,pci,pc: features, fixes and cleanups
> >
> > - a new vhost crypto device
> > - new stats in virtio balloon
> > - virtio eventfd rework for boot speedup
> > - vhost memory rework for boot speedup
> > - fixes and cleanups all over the place
> >
> > Signed-off-by: Michael S. Tsirkin 
> >
> 
> Hi. This has some format-string issues:
> 
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_start':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:112:26:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>  error_report("failed to init vhost_crypto for queue %lu", i);
>   ^
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_init':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:205:40:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>  cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
> ^
> 
> thanks
> -- PMM

Fixed up now, new hash d94c5ae38f25a6e77e07007dcd510b7203a687e8

-- 
MST

Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups

[Peter Maydell]

On 8 February 2018 at 19:08, Michael S. Tsirkin  wrote:
> The following changes since commit 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
>
>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into 
> staging (2018-02-08 14:31:51 +)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:
>
>   virtio-balloon: include statistics of disk/file caches (2018-02-08 21:06:42 
> +0200)
>
> 
> virtio,vhost,pci,pc: features, fixes and cleanups
>
> - a new vhost crypto device
> - new stats in virtio balloon
> - virtio eventfd rework for boot speedup
> - vhost memory rework for boot speedup
> - fixes and cleanups all over the place
>
> Signed-off-by: Michael S. Tsirkin 
>

Hi. This has some format-string issues:

/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
'cryptodev_vhost_user_start':
/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:112:26:
error: format '%lu' expects argument of type 'long unsigned int', but
argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
 error_report("failed to init vhost_crypto for queue %lu", i);
  ^
/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
'cryptodev_vhost_user_init':
/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:205:40:
error: format '%lu' expects argument of type 'long unsigned int', but
argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
 cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
^

thanks
-- PMM