On Sun, Sep 27, 2015 at 5:13 PM, Marc-André Lureau
<marcandre.lur...@gmail.com> wrote:
> Hi
>
> On Sun, Sep 27, 2015 at 3:12 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>> On Thu, Sep 24, 2015 at 05:53:05PM -0400, Marc-André Lureau wrote:
>>> Hi
>>>
>>> ----- Original Message -----
>>> > On Thu, Sep 24, 2015 at 6:22 PM, <marcandre.lur...@redhat.com> wrote:
>>> > > From: Thibaut Collet <thibaut.col...@6wind.com>
>>> > >
>>> > > A new vhost user message is added to allow QEMU to ask to vhost user
>>> > > backend to
>>> > > broadcast a fake RARP after live migration for guest without
>>> > > GUEST_ANNOUNCE
>>> > > capability.
>>> > >
>>> > > This new message is sent only if the backend supports the new
>>> > > VHOST_USER_PROTOCOL_F_RARP protocol feature.
>>> > > The payload of this new message is the MAC address of the guest (not
>>> > > known
>>> > > by
>>> > > the backend). The MAC address is copied in the first 6 bytes of a u64 to
>>> > > avoid
>>> > > to create a new payload message type.
>>> > >
>>> > > This new message has no equivalent ioctl so a new callback is added in
>>> > > the
>>> > > userOps structure to send the request.
>>> > >
>>> > > Upon reception of this new message the vhost user backend must generate
>>> > > and
>>> > > broadcast a fake RARP request to notify the migration is terminated.
>>> > >
>>> > > Signed-off-by: Thibaut Collet <thibaut.col...@6wind.com>
>>> > > [Rebased and fixed checkpatch errors - Marc-André]
>>> > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>>> > > ---
>>> > > docs/specs/vhost-user.txt | 15 +++++++++++++++
>>> > > hw/net/vhost_net.c | 17 +++++++++++++++++
>>> > > hw/virtio/vhost-user.c | 30 ++++++++++++++++++++++++++++++
>>> > > include/hw/virtio/vhost-backend.h | 3 +++
>>> > > include/net/vhost_net.h | 1 +
>>> > > net/vhost-user.c | 24 ++++++++++++++++++++++--
>>> > > 6 files changed, 88 insertions(+), 2 deletions(-)
>>> > >
>>> > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>>> > > index e0292a0..e0d71e2 100644
>>> > > --- a/docs/specs/vhost-user.txt
>>> > > +++ b/docs/specs/vhost-user.txt
>>> > > @@ -194,6 +194,7 @@ Protocol features
>>> > >
>>> > > #define VHOST_USER_PROTOCOL_F_MQ 0
>>> > > #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
>>> > > +#define VHOST_USER_PROTOCOL_F_RARP 2
>>> > >
>>> > > Message types
>>> > > -------------
>>> > > @@ -381,3 +382,17 @@ Message types
>>> > > Master payload: vring state description
>>> > >
>>> > > Signal slave to enable or disable corresponding vring.
>>> > > +
>>> > > + * VHOST_USER_SEND_RARP
>>> > > +
>>> > > + Id: 19
>>> > > + Equivalent ioctl: N/A
>>> > > + Master payload: u64
>>> > > +
>>> > > + Ask vhost user backend to broadcast a fake RARP to notify the
>>> > > migration
>>> > > + is terminated for guest that does not support GUEST_ANNOUNCE.
>>> > > + Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is
>>> > > present
>>> > > in
>>> > > + VHOST_USER_GET_FEATURES and protocol feature bit
>>> > > VHOST_USER_PROTOCOL_F_RARP
>>> > > + is present in VHOST_USER_GET_PROTOCOL_FEATURES.
>>> > > + The first 6 bytes of the payload contain the mac address of the
>>> > > guest to
>>> > > + allow the vhost user backend to construct and broadcast the fake
>>> > > RARP.
>>> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>>> > > index 840f443..da66b64 100644
>>> > > --- a/hw/net/vhost_net.c
>>> > > +++ b/hw/net/vhost_net.c
>>> > > @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
>>> > > g_free(net);
>>> > > }
>>> > >
>>> > > +int vhost_net_notify_migration_done(struct vhost_net *net, char*
>>> > > mac_addr)
>>> > > +{
>>> > > + const VhostOps *vhost_ops = net->dev.vhost_ops;
>>> > > + int r = -1;
>>> > > +
>>> > > + if (vhost_ops->vhost_migration_done) {
>>> > > + r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
>>> > > + }
>>> > > +
>>> > > + return r;
>>> > > +}
>>> > > +
>>> > > bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>>> > > {
>>> > > return vhost_virtqueue_pending(&net->dev, idx);
>>> > > @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
>>> > > VirtIODevice *dev,
>>> > > {
>>> > > }
>>> > >
>>> > > +int vhost_net_notify_migration_done(struct vhost_net *net)
>>> > > +{
>>> > > + return -1;
>>> > > +}
>>> > > +
>>> > > VHostNetState *get_vhost_net(NetClientState *nc)
>>> > > {
>>> > > return 0;
>>> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>> > > index 455caba..b7f3699 100644
>>> > > --- a/hw/virtio/vhost-user.c
>>> > > +++ b/hw/virtio/vhost-user.c
>>> > > @@ -10,6 +10,7 @@
>>> > >
>>> > > #include "hw/virtio/vhost.h"
>>> > > #include "hw/virtio/vhost-backend.h"
>>> > > +#include "hw/virtio/virtio-net.h"
>>> > > #include "sysemu/char.h"
>>> > > #include "sysemu/kvm.h"
>>> > > #include "qemu/error-report.h"
>>> > > @@ -30,6 +31,7 @@
>>> > > #define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
>>> > > #define VHOST_USER_PROTOCOL_F_MQ 0
>>> > > #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
>>> > > +#define VHOST_USER_PROTOCOL_F_RARP 2
>>> >
>>> > The VHOST_USER_PROTOCOL_FEATURE_MASK must be changed and set to 0x7ULL
>>> >
>> Better, change VHOST_USER_PROTOCOL_FEATURE_MASK to be
>> calculated based on other macros.
>
> yeah, I actually thought about something like this a few days ago:
> https://github.com/elmarco/qemu/commit/b4e4ec2e4c78f6b12e34822d8a3f3a101d065d80
>
I agree an automatic computation of the FEATURE_MASK from the
different protocol feature will avoid issues wiht rebase or merge
operation.
Maybe a solution like that is clearer ?
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x7ULL
-#define VHOST_USER_PROTOCOL_F_MQ 0
-#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
-#define VHOST_USER_PROTOCOL_F_RARP 2
+typedef enum VhostUserProtocolFeature {
+VHOST_USER_PROTOCOL_F_MQ = 0,
+VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
+VHOST_USER_PROTOCOL_F_RARP = 2,
+VHOST_USER_PROTOCOL_F_MAX
+} VhostUserProtocolFeature ;
+#define VHOST_USER_PROTOCOL_FEATURE_MASK ((1 << VHOST_USER_PROTOCOL_F_MAX) - 1)
> Is that what you have in mind or rather make a bit mask of all the
> features? Maybe the bitmask of all the features is simpler.
>
>>
>>>
>>> Good catch (too many rebases, having a test would help to prevent this kind
>>> of mistake)
>>>
>>> thanks
>>
>> So there will be v6 with a fix?
>
> If there is no further changes in the series, I guess you could fix it
> when cherry-picking. But if I have to send a v6, I'll include the fix.
>
> thanks
>
>
>
> --
> Marc-André Lureau
