Andreas Färber <afaer...@suse.de> wrote: > From: Andreas Färber <andreas.faer...@web.de> > > VMState supports the type bool but qdev instead supports bit, backed by > uint32_t. Therefore let's add PROP_TYPE_BOOL and qdev_prop_set_bool(). > PropertyInfo qdev_prop_bit = { > - .name = "boolean", > + .name = "bit",
my plan for vmstate was to name this "bool32", as it feels more descriptive (on QDEV, we can change it, but for qemu for compatibility reasons, it needs to stay as 4 bytes on the wire, but we can have a bool on the struct). This are all the uses of DEFINE_PROP_BIT. ./hw/scsi-disk.c:1736: DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false), Con be movde to bool ./hw/scsi-disk.c:1780: DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false), same ./hw/usb-msd.c:658: DEFINE_PROP_BIT("removable", MSDState, removable, 0, false), s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable, s->conf.bootindex); *should* be moved to bool ./hw/hpet.c:707: DEFINE_PROP_BIT("msi", HPETState, flags, HPET_MSI_SUPPORT, false), it is a bitmap with only one bit defined, not sure about this one. ./hw/virtio.h:213: DEFINE_PROP_BIT("indirect_desc", _state, _field, \ ./hw/virtio.h:215: DEFINE_PROP_BIT("event_idx", _state, _field, \ This is a real bitmap with 2 values. ./hw/virtio-pci.c:800: DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, ./hw/virtio-pci.c:819: DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, ./hw/virtio-pci.c:843: DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, ./hw/9pfs/virtio-9p-device.c:175: DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, Another bitmap with only one defined value. ./hw/virtio-blk.h:103: DEFINE_PROP_BIT("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true) It is a bitmap ./hw/ivshmem.c:782: DEFINE_PROP_BIT("ioeventfd", IVShmemState, features, IVSHMEM_IOEVENTFD, false), ./hw/ivshmem.c:783: DEFINE_PROP_BIT("msi", IVShmemState, features, IVSHMEM_MSI, true), Another bitmap with two values. ./hw/virtio-net.h:172: DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \ ./hw/virtio-net.h:173: DEFINE_PROP_BIT("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \ ./hw/virtio-net.h:174: DEFINE_PROP_BIT("gso", _state, _field, VIRTIO_NET_F_GSO, true), \ ./hw/virtio-net.h:175: DEFINE_PROP_BIT("guest_tso4", _state, _field, VIRTIO_NET_F_GUEST_TSO4, true), \ ./hw/virtio-net.h:176: DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \ ./hw/virtio-net.h:177: DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \ ./hw/virtio-net.h:178: DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \ ./hw/virtio-net.h:179: DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \ ./hw/virtio-net.h:180: DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \ ./hw/virtio-net.h:181: DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \ ./hw/virtio-net.h:182: DEFINE_PROP_BIT("host_ufo", _state, _field, VIRTIO_NET_F_HOST_UFO, true), \ ./hw/virtio-net.h:183: DEFINE_PROP_BIT("mrg_rxbuf", _state, _field, VIRTIO_NET_F_MRG_RXBUF, true), \ ./hw/virtio-net.h:184: DEFINE_PROP_BIT("status", _state, _field, VIRTIO_NET_F_STATUS, true), \ ./hw/virtio-net.h:185: DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \ ./hw/virtio-net.h:186: DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \ ./hw/virtio-net.h:187: DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \ ./hw/virtio-net.h:188: DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true) Bitmap. ./hw/i8259.c:570: DEFINE_PROP_BIT("master", PicState, master, 0, false), BOOL ./hw/pci.c:58: DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, ./hw/pci.c:60: DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present, bitmap with two values. ./hw/pxa2xx_timer.c:488: DEFINE_PROP_BIT("tm4", PXA2xxTimerInfo, flags, ./hw/pxa2xx_timer.c:502: DEFINE_PROP_BIT("tm4", PXA2xxTimerInfo, flags, Bitmap with only one value. > +{ > + bool *ptr = qdev_get_prop_ptr(dev, prop); > + if (strcmp(str, "true") == 0 || strcmp(str, "yes") == 0) { > + *ptr = true; > + } else if (strcmp(str, "false") == 0 || strcmp(str, "no") == 0) { > + *ptr = false; > + } else { > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int parse_bool_switch(DeviceState *dev, Property *prop, > + const char *str) > +{ > + bool *ptr = qdev_get_prop_ptr(dev, prop); > + if (strcmp(str, "on") == 0) { > + *ptr = true; > + } else if (strcmp(str, "off") == 0) { > + *ptr = false; > + } else { > + return -EINVAL; > + } > + > + return 0; > +} As I am joining late to this discussion, I am not going to point it very strong. But I think that it is an easy to have a single bool type that accept yes/on/true and no/off/false. Didn't really see a strong advantage with the split. Later, Juan.