[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net: Remove vhostforce option in addition to vh
From: |
Pankaj Gupta |
Subject: |
Re: [Qemu-devel] [PATCH] net: Remove vhostforce option in addition to vhost parameter |
Date: |
Fri, 29 May 2015 00:59:58 -0400 (EDT) |
> >
> > Ping.
> >
> > Can I get any suggestions on this patch.
> >
> > Best regards,
> > Pankaj
>
>
>
> > >
> > > vhostforce was added to enable vhost when
> > > guest don't have MSI-X support.
> > > Now, we have scenarios like DPDK in Guest which dont use
> > > interrupts and still use vhost. Also, performance of guests
> > > without MSI-X support is getting less popular.
> > >
> > > Its OK to remove this extra option and enable vhost
> > > on the basis of vhost=ON/OFF.
> > > Done basic testing with vhost on/off for latest guests
> > > and old guests(non-msix).
> > >
> > > Signed-off-by: Pankaj Gupta <address@hidden>
>
> I think this silently changes the command line semantics:
> previously vhostforce=on implies vhost=on, with this patch
> it does not.
Yes, we are trying to eliminate 'vhostforce'. As we already have vhost='ON/OFF'.
In case of 'virtio-net', with vhost= 'ON', without guest support of MSI-X we
will fall
back to QEMU. If we use vhostforce='ON', we will use 'vhost' even it has to
follow a long path
again via QEMU->vhost. also we don't have support of KVM level triggered irqfd.
In case 'vhost-user' is used, we are hard-coding 'vhostforce' to 'true' because
we want
vhost(vhost-user) to run. We are also removing this as it alos checks
'vhost_dev_query'.
Any use-case where we want to forcefully use vhost even we are falling back to
userspace Virtio-net?
One use-case I can think for 'vhost-user' without MSI-X support e.g iPXE boot.
Here we want to use vhost-user
for every/any condition, because we don't have any alternative.
Any suggestions?
>
> > > ---
> > > hw/net/vhost_net.c | 2 +-
> > > hw/scsi/vhost-scsi.c | 2 +-
> > > hw/virtio/vhost.c | 6 ++----
> > > include/hw/virtio/vhost.h | 3 +--
> > > include/net/vhost_net.h | 1 -
> > > net/tap.c | 4 +---
> > > net/vhost-user.c | 1 -
> > > 7 files changed, 6 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 4e3a061..7139b9f 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -159,7 +159,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions
> > > *options)
> > > net->dev.vqs = net->vqs;
> > >
> > > r = vhost_dev_init(&net->dev, options->opaque,
> > > - options->backend_type, options->force);
> > > + options->backend_type);
> > > if (r < 0) {
> > > goto fail;
> > > }
> > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> > > index 618b0af..b15390e 100644
> > > --- a/hw/scsi/vhost-scsi.c
> > > +++ b/hw/scsi/vhost-scsi.c
> > > @@ -246,7 +246,7 @@ static void vhost_scsi_realize(DeviceState *dev,
> > > Error
> > > **errp)
> > > s->dev.backend_features = 0;
> > >
> > > ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd,
> > > - VHOST_BACKEND_TYPE_KERNEL, true);
> > > + VHOST_BACKEND_TYPE_KERNEL);
> > > if (ret < 0) {
> > > error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
> > > strerror(-ret));
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index 5a12861..ce33e04 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -811,7 +811,7 @@ static void vhost_virtqueue_cleanup(struct
> > > vhost_virtqueue *vq)
> > > }
> > >
> > > int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> > > - VhostBackendType backend_type, bool force)
> > > + VhostBackendType backend_type)
> > > {
> > > uint64_t features;
> > > int i, r;
> > > @@ -874,7 +874,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void
> > > *opaque,
> > > hdev->started = false;
> > > hdev->memory_changed = false;
> > > memory_listener_register(&hdev->memory_listener,
> > > &address_space_memory);
> > > - hdev->force = force;
> > > return 0;
> > > fail_vq:
> > > while (--i >= 0) {
> > > @@ -909,8 +908,7 @@ bool vhost_dev_query(struct vhost_dev *hdev,
> > > VirtIODevice
> > > *vdev)
> > > VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> > >
> > > return !k->query_guest_notifiers ||
> > > - k->query_guest_notifiers(qbus->parent) ||
> > > - hdev->force;
> > > + k->query_guest_notifiers(qbus->parent);
> > > }
> > >
> > > /* Stop processing guest IO notifications in qemu.
> > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > > index d5593d1..27eae3e 100644
> > > --- a/include/hw/virtio/vhost.h
> > > +++ b/include/hw/virtio/vhost.h
> > > @@ -46,7 +46,6 @@ struct vhost_dev {
> > > vhost_log_chunk_t *log;
> > > unsigned long long log_size;
> > > Error *migration_blocker;
> > > - bool force;
> > > bool memory_changed;
> > > hwaddr mem_changed_start_addr;
> > > hwaddr mem_changed_end_addr;
> > > @@ -55,7 +54,7 @@ struct vhost_dev {
> > > };
> > >
> > > int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> > > - VhostBackendType backend_type, bool force);
> > > + VhostBackendType backend_type);
> > > void vhost_dev_cleanup(struct vhost_dev *hdev);
> > > bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
> > > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
> > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> > > index b1c18a3..966a945 100644
> > > --- a/include/net/vhost_net.h
> > > +++ b/include/net/vhost_net.h
> > > @@ -11,7 +11,6 @@ typedef struct VhostNetOptions {
> > > VhostBackendType backend_type;
> > > NetClientState *net_backend;
> > > void *opaque;
> > > - bool force;
> > > } VhostNetOptions;
> > >
> > > struct vhost_net *vhost_net_init(VhostNetOptions *options);
> > > diff --git a/net/tap.c b/net/tap.c
> > > index 968df46..b9002f7 100644
> > > --- a/net/tap.c
> > > +++ b/net/tap.c
> > > @@ -635,13 +635,11 @@ static int net_init_tap_one(const NetdevTapOptions
> > > *tap, NetClientState *peer,
> > > }
> > > }
> > >
> > > - if (tap->has_vhost ? tap->vhost :
> > > - vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
> > > + if (tap->has_vhost ? tap->vhost : vhostfdname) {
> > > VhostNetOptions options;
> > >
> > > options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
> > > options.net_backend = &s->nc;
> > > - options.force = tap->has_vhostforce && tap->vhostforce;
> > >
> > > if (tap->has_vhostfd || tap->has_vhostfds) {
> > > vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err);
> > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > > index 24e050c..9966de5 100644
> > > --- a/net/vhost-user.c
> > > +++ b/net/vhost-user.c
> > > @@ -51,7 +51,6 @@ static int vhost_user_start(VhostUserState *s)
> > > options.backend_type = VHOST_BACKEND_TYPE_USER;
> > > options.net_backend = &s->nc;
> > > options.opaque = s->chr;
> > > - options.force = s->vhostforce;
> > >
> > > s->vhost_net = vhost_net_init(&options);
> > >
> > > --
> > > 1.8.3.1
> > >
> > >
> > >
>
>
Re: [Qemu-devel] [PATCH] net: Remove vhostforce option in addition to vhost parameter, Michael S. Tsirkin, 2015/05/27
- Re: [Qemu-devel] [PATCH] net: Remove vhostforce option in addition to vhost parameter,
Pankaj Gupta <=