(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'.
They are packed, because both are used to serialize/deserialize the stream.
That's a bit unfortunate for two structures with the same name.
Yes, maybe it's time to have a canonical system header used by both?
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?
I believe the extra fields are not used for serializing the message, but just a convenient way to group related data.
Is it safe for me to add a new, larger entry in the payload union
without breaking existing clients?
It should be.
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.
Indeed, we probably want to allocate the message separately then.
thanks