From: Liming Wu <[email protected]>
Need to insmod vhost_test.ko before run virtio_test.
Give some hints to users.
Signed-off-by: Liming Wu <[email protected]>
---
tools/virtio/virtio_test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 028f54e6854a..ce2c4d93d735 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -135,6 +135,10 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
dev->buf = malloc(dev->buf_size);
assert(dev->buf);
dev->control = open("/dev/vhost-test", O_RDWR);
+
+ if (dev->control < 0)
+ fprintf(stderr, "Install vhost_test module" \
+ "(./vhost_test/vhost_test.ko) firstly\n");
assert(dev->control >= 0);
r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
assert(r >= 0);
--
2.34.1
On Tue, Sep 26, 2023 at 01:00:20PM +0800, [email protected] wrote:
> From: Liming Wu <[email protected]>
>
> Need to insmod vhost_test.ko before run virtio_test.
> Give some hints to users.
>
> Signed-off-by: Liming Wu <[email protected]>
> ---
> tools/virtio/virtio_test.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> index 028f54e6854a..ce2c4d93d735 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -135,6 +135,10 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
> dev->buf = malloc(dev->buf_size);
> assert(dev->buf);
> dev->control = open("/dev/vhost-test", O_RDWR);
> +
> + if (dev->control < 0)
> + fprintf(stderr, "Install vhost_test module" \
> + "(./vhost_test/vhost_test.ko) firstly\n");
Thanks!
things to improve:
firstly -> first
add space before (
End sentence with a dot
align "" on the two lines
> assert(dev->control >= 0);
> r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
> assert(r >= 0);
> --
> 2.34.1
From: Liming Wu <[email protected]>
Fixes: 8bd2f71054bd ("virtio_ring: introduce dma sync api for virtqueue")
also add dma sync api for virtio test.
Signed-off-by: Liming Wu <[email protected]>
---
tools/virtio/linux/dma-mapping.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/virtio/linux/dma-mapping.h b/tools/virtio/linux/dma-mapping.h
index 834a90bd3270..822ecaa8e4df 100644
--- a/tools/virtio/linux/dma-mapping.h
+++ b/tools/virtio/linux/dma-mapping.h
@@ -24,11 +24,23 @@ enum dma_data_direction {
#define dma_map_page(d, p, o, s, dir) (page_to_phys(p) + (o))
#define dma_map_single(d, p, s, dir) (virt_to_phys(p))
+#define dma_map_single_attrs(d, p, s, dir, a) (virt_to_phys(p))
#define dma_mapping_error(...) (0)
#define dma_unmap_single(d, a, s, r) do { (void)(d); (void)(a); (void)(s); (void)(r); } while (0)
#define dma_unmap_page(d, a, s, r) do { (void)(d); (void)(a); (void)(s); (void)(r); } while (0)
+#define sg_dma_address(sg) (0)
+#define dma_need_sync(v, a) (0)
+#define dma_unmap_single_attrs(d, a, s, r, t) do { \
+ (void)(d); (void)(a); (void)(s); (void)(r); (void)(t); \
+} while (0)
+#define dma_sync_single_range_for_cpu(d, a, o, s, r) do { \
+ (void)(d); (void)(a); (void)(o); (void)(s); (void)(r); \
+} while (0)
+#define dma_sync_single_range_for_device(d, a, o, s, r) do { \
+ (void)(d); (void)(a); (void)(o); (void)(s); (void)(r); \
+} while (0)
#define dma_max_mapping_size(...) SIZE_MAX
#endif
--
2.34.1
From: Liming Wu <[email protected]>
Need to insmod vhost_test.ko before run virtio_test.
Give some hints to users.
Signed-off-by: Liming Wu <[email protected]>
---
tools/virtio/virtio_test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 028f54e6854a..13572a677c35 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -135,6 +135,10 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
dev->buf = malloc(dev->buf_size);
assert(dev->buf);
dev->control = open("/dev/vhost-test", O_RDWR);
+
+ if (dev->control < 0)
+ fprintf(stderr,
+ "Install vhost_test module (/vhost_test/vhost_test.ko) first.\n");
assert(dev->control >= 0);
r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
assert(r >= 0);
--
2.34.1
On Sat, 7 Oct 2023 14:55:46 +0800, [email protected] wrote:
> From: Liming Wu <[email protected]>
>
> Fixes: 8bd2f71054bd ("virtio_ring: introduce dma sync api for virtqueue")
> also add dma sync api for virtio test.
>
> Signed-off-by: Liming Wu <[email protected]>
You should post a new thread.
Thanks.
> ---
> tools/virtio/linux/dma-mapping.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tools/virtio/linux/dma-mapping.h b/tools/virtio/linux/dma-mapping.h
> index 834a90bd3270..822ecaa8e4df 100644
> --- a/tools/virtio/linux/dma-mapping.h
> +++ b/tools/virtio/linux/dma-mapping.h
> @@ -24,11 +24,23 @@ enum dma_data_direction {
> #define dma_map_page(d, p, o, s, dir) (page_to_phys(p) + (o))
>
> #define dma_map_single(d, p, s, dir) (virt_to_phys(p))
> +#define dma_map_single_attrs(d, p, s, dir, a) (virt_to_phys(p))
> #define dma_mapping_error(...) (0)
>
> #define dma_unmap_single(d, a, s, r) do { (void)(d); (void)(a); (void)(s); (void)(r); } while (0)
> #define dma_unmap_page(d, a, s, r) do { (void)(d); (void)(a); (void)(s); (void)(r); } while (0)
>
> +#define sg_dma_address(sg) (0)
> +#define dma_need_sync(v, a) (0)
> +#define dma_unmap_single_attrs(d, a, s, r, t) do { \
> + (void)(d); (void)(a); (void)(s); (void)(r); (void)(t); \
> +} while (0)
> +#define dma_sync_single_range_for_cpu(d, a, o, s, r) do { \
> + (void)(d); (void)(a); (void)(o); (void)(s); (void)(r); \
> +} while (0)
> +#define dma_sync_single_range_for_device(d, a, o, s, r) do { \
> + (void)(d); (void)(a); (void)(o); (void)(s); (void)(r); \
> +} while (0)
> #define dma_max_mapping_size(...) SIZE_MAX
>
> #endif
> --
> 2.34.1
>
> -----Original Message-----
> From: Xuan Zhuo <[email protected]>
> Sent: Sunday, October 8, 2023 10:19 AM
> To: Liming Wu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Liming Wu
> <[email protected]>; Michael S . Tsirkin <[email protected]>;
> Jason Wang <[email protected]>
> Subject: Re: [PATCH v2 1/2] tools/virtio: Add dma sync api for virtio test
>
> On Sat, 7 Oct 2023 14:55:46 +0800, [email protected] wrote:
> > From: Liming Wu <[email protected]>
> >
> > Fixes: 8bd2f71054bd ("virtio_ring: introduce dma sync api for
> > virtqueue") also add dma sync api for virtio test.
> >
> > Signed-off-by: Liming Wu <[email protected]>
>
> You should post a new thread.
OK, Thanks.
>
> Thanks.
>
>
> > ---
> >
On Tue, Sep 26, 2023 at 1:00 PM <[email protected]> wrote:
>
> From: Liming Wu <[email protected]>
>
> Need to insmod vhost_test.ko before run virtio_test.
> Give some hints to users.
>
> Signed-off-by: Liming Wu <[email protected]>
> ---
> tools/virtio/virtio_test.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> index 028f54e6854a..ce2c4d93d735 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -135,6 +135,10 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
> dev->buf = malloc(dev->buf_size);
> assert(dev->buf);
> dev->control = open("/dev/vhost-test", O_RDWR);
> +
> + if (dev->control < 0)
> + fprintf(stderr, "Install vhost_test module" \
> + "(./vhost_test/vhost_test.ko) firstly\n");
There should be many other reasons to fail for open().
Let's use strerror()?
Thanks
> assert(dev->control >= 0);
> r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
> assert(r >= 0);
> --
> 2.34.1
>
> -----Original Message-----
> From: Jason Wang <[email protected]>
> Sent: Sunday, October 8, 2023 12:36 PM
> To: Liming Wu <[email protected]>
> Cc: Michael S . Tsirkin <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 2/2] tools/virtio: Add hints when module is not installed
>
> On Tue, Sep 26, 2023 at 1:00 PM <[email protected]> wrote:
> >
> > From: Liming Wu <[email protected]>
> >
> > Need to insmod vhost_test.ko before run virtio_test.
> > Give some hints to users.
> >
> > Signed-off-by: Liming Wu <[email protected]>
> > ---
> > tools/virtio/virtio_test.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> > index 028f54e6854a..ce2c4d93d735 100644
> > --- a/tools/virtio/virtio_test.c
> > +++ b/tools/virtio/virtio_test.c
> > @@ -135,6 +135,10 @@ static void vdev_info_init(struct vdev_info* dev,
> unsigned long long features)
> > dev->buf = malloc(dev->buf_size);
> > assert(dev->buf);
> > dev->control = open("/dev/vhost-test", O_RDWR);
> > +
> > + if (dev->control < 0)
> > + fprintf(stderr, "Install vhost_test module" \
> > + "(./vhost_test/vhost_test.ko) firstly\n");
>
> There should be many other reasons to fail for open().
>
> Let's use strerror()?
Yes, Thanks for the review.
Please rechecked the code as follow:
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -135,6 +135,11 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
dev->buf = malloc(dev->buf_size);
assert(dev->buf);
dev->control = open("/dev/vhost-test", O_RDWR);
+
+ if (dev->control == NULL)
+ fprintf(stderr,
+ "%s: Check whether vhost_test.ko is installed.\n",
+ strerror(errno));
assert(dev->control >= 0);
r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
assert(r >= 0);
Thanks
On Mon, Oct 09, 2023 at 02:44:55AM +0000, Liming Wu wrote:
>
>
> > -----Original Message-----
> > From: Jason Wang <[email protected]>
> > Sent: Sunday, October 8, 2023 12:36 PM
> > To: Liming Wu <[email protected]>
> > Cc: Michael S . Tsirkin <[email protected]>; [email protected];
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 2/2] tools/virtio: Add hints when module is not installed
> >
> > On Tue, Sep 26, 2023 at 1:00 PM <[email protected]> wrote:
> > >
> > > From: Liming Wu <[email protected]>
> > >
> > > Need to insmod vhost_test.ko before run virtio_test.
> > > Give some hints to users.
> > >
> > > Signed-off-by: Liming Wu <[email protected]>
> > > ---
> > > tools/virtio/virtio_test.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> > > index 028f54e6854a..ce2c4d93d735 100644
> > > --- a/tools/virtio/virtio_test.c
> > > +++ b/tools/virtio/virtio_test.c
> > > @@ -135,6 +135,10 @@ static void vdev_info_init(struct vdev_info* dev,
> > unsigned long long features)
> > > dev->buf = malloc(dev->buf_size);
> > > assert(dev->buf);
> > > dev->control = open("/dev/vhost-test", O_RDWR);
> > > +
> > > + if (dev->control < 0)
> > > + fprintf(stderr, "Install vhost_test module" \
> > > + "(./vhost_test/vhost_test.ko) firstly\n");
> >
> > There should be many other reasons to fail for open().
> >
> > Let's use strerror()?
> Yes, Thanks for the review.
> Please rechecked the code as follow:
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -135,6 +135,11 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
> dev->buf = malloc(dev->buf_size);
> assert(dev->buf);
> dev->control = open("/dev/vhost-test", O_RDWR);
> +
> + if (dev->control == NULL)
???
Why are you comparing a file descriptor to NULL?
> + fprintf(stderr,
> + "%s: Check whether vhost_test.ko is installed.\n",
> + strerror(errno));
No, do not suggest checking unconditionally this is just wasting user's
time. You would have to check the exact errno value. You will either
get ENOENT or ENODEV if module is not loaded. Other errors indicate
other problems. And what matters is whether it's loaded, not installed
- vhost_test.ko will not get auto-loaded even if installed.
> assert(dev->control >= 0);
> r = ioctl(dev->control, VHOST_SET_OWNER, NULL);
> assert(r >= 0);
>
> Thanks
>
In short, I am not applying this patch. If you really want to make
things a bit easier in case of errors, replace all assert r >= 0 with
a macro that prints out strerror(errno), that should be enough.
Maybe print file/line number too while we are at it.
--
MST