qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: marcandre.lureau@redhat.com, mst@redhat.com
Cc: vgoyal@redhat.com, stefanha@redhat.com, qemu-devel@nongnu.org
Subject: vhost-user payload union restrictions ?
Date: Wed, 5 May 2021 13:36:34 +0100	[thread overview]
Message-ID: <YJKRUp8E+J7OSCgg@work-vm> (raw)

(Resend, remembering to add list)
Hi,
  I'm trying to understand what restrictions there are on the
payload that's part of VhostUserMsg; and am confused by
inconsistencies.

Lets start with this version:

subprojects/libvhost-user/libvhost-user.h :
typedef struct VhostUserMsg {
    int request;

#define VHOST_USER_VERSION_MASK     (0x3)
#define VHOST_USER_REPLY_MASK       (0x1 << 2)
#define VHOST_USER_NEED_REPLY_MASK  (0x1 << 3)
    uint32_t flags;
    uint32_t size; /* the following payload size */

    union {
#define VHOST_USER_VRING_IDX_MASK   (0xff)
#define VHOST_USER_VRING_NOFD_MASK  (0x1 << 8)
        uint64_t u64;
        struct vhost_vring_state state;
        struct vhost_vring_addr addr;
        VhostUserMemory memory;
        VhostUserMemRegMsg memreg;
        VhostUserLog log;
        VhostUserConfig config;
        VhostUserVringArea area;
        VhostUserInflight inflight;
    } payload;

    int fds[VHOST_MEMORY_BASELINE_NREGIONS];
    int fd_num;
    uint8_t *data;
} VU_PACKED VhostUserMsg;

note the 'fds' array after the payload but before
the end of the structure.

But then there's the version in:
hw/virtio/vhost-user.c
typedef union {
#define VHOST_USER_VRING_IDX_MASK   (0xff)
#define VHOST_USER_VRING_NOFD_MASK  (0x1<<8)
        uint64_t u64;
        struct vhost_vring_state state;
        struct vhost_vring_addr addr;
        VhostUserMemory memory;
        VhostUserMemRegMsg mem_reg;
        VhostUserLog log;
        struct vhost_iotlb_msg iotlb;
        VhostUserConfig config;
        VhostUserCryptoSession session;
        VhostUserVringArea area;
        VhostUserInflight inflight;
} VhostUserPayload;

typedef struct VhostUserMsg {
    VhostUserHeader hdr;
    VhostUserPayload payload;
} QEMU_PACKED VhostUserMsg;

which hasn't got the 'fds' section.
Yet they're both marked as 'packed'.

That's a bit unfortunate for two structures with the same name.

Am I right in thinking that the vhost-user.c version is sent over
the wire, while the libvhost-user.h one is really just an interface?

Is it safe for me to add a new, larger entry in the payload union
without breaking existing clients?

I ended up at this question after trying to add a variable length
entry to the union:

typedef struct {
    VhostUserFSSlaveMsg fs;
    VhostUserFSSlaveMsgEntry entries[VHOST_USER_FS_SLAVE_MAX_ENTRIES];
} QEMU_PACKED VhostUserFSSlaveMsgMax;

...
union ....
        VhostUserFSSlaveMsg fs;
        VhostUserFSSlaveMsgMax fs_max; /* Never actually used */
} VhostUserPayload;

and in the .h I had:
typedef struct {
    /* Generic flags for the overall message */
    uint32_t flags;
    /* Number of entries */
    uint16_t count;
    /* Spare */
    uint16_t align;

    VhostUserFSSlaveMsgEntry entries[];
} VhostUserFSSlaveMsg;

    union {
...
        VhostUserInflight inflight;
        VhostUserFSSlaveMsg fs;
    } payload;

which is apparently OK in the .c version, and gcc is happy with the same
in the libvhost-user.h version; but clang gets upset by the .h
version because it doesn't like the variable length structure
before the end of the struct - which I have sympathy for.

Dave

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



             reply	other threads:[~2021-05-05 12:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 12:36 Dr. David Alan Gilbert [this message]
2021-05-05 12:59 ` vhost-user payload union restrictions ? Marc-André Lureau
2021-05-05 17:54   ` Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJKRUp8E+J7OSCgg@work-vm \
    --to=dgilbert@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).