Re: [Xen-devel] [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and 'xen-cdrom'
> -Original Message- > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > Sent: 07 December 2018 15:26 > To: Paul Durrant > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; xen- > de...@lists.xenproject.org; Kevin Wolf ; Max Reitz > ; Stefano Stabellini > Subject: Re: [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and > 'xen-cdrom' > > On Fri, Dec 07, 2018 at 02:39:40PM +, Paul Durrant wrote: > > > -Original Message- > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > > > Sent: 07 December 2018 14:35 > > > To: Paul Durrant > > > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; xen- > > > de...@lists.xenproject.org; Kevin Wolf ; Max Reitz > > > ; Stefano Stabellini > > > Subject: Re: [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' > and > > > 'xen-cdrom' > > > > > > On Thu, Dec 06, 2018 at 03:08:29PM +, Paul Durrant wrote: > > > > +static char *disk_to_vbd_name(unsigned int disk) > > > > +{ > > > > +char *name, *prefix = (disk >= 26) ? > > > > +disk_to_vbd_name((disk / 26) - 1) : g_strdup(""); > > > > + > > > > +name = g_strdup_printf("%s%c", prefix, 'a' + disk); > > > > > > I don't think that works, if disk is 27, we do ('a' + 27) here. It's > > > probably missing a `disk % 26`. > > > > Damn, yes I was not allowing the >2 letters. > > > > > > > > > +g_free(prefix); > > > > + > > > > +return name; > > > > +} > > > > > > [...] > > > > > > > +static unsigned int vbd_name_to_disk(const char *name, const char > > > **endp) > > > > +{ > > > > +unsigned int disk = 0; > > > > + > > > > +while (*name != '\0') { > > > > +if (!g_ascii_isalpha(*name) || !g_ascii_islower(*name)) { > > > > +break; > > > > +} > > > > + > > > > +disk *= 26; > > > > +disk += *name++ - 'a'; > > > > +} > > > > +*endp = name; > > > > + > > > > +return disk; > > > > +} > > > > + > > > > +static void xen_block_set_vdev(Object *obj, Visitor *v, const char > > > *name, > > > > + void *opaque, Error **errp) > > > > +{ > > > > > > Setting vdev doesn't work. I've tried to add a disk `xvdaa', and it > > > result in `xvda', or `d0p0' (in the trace). (Same result with > `xvdaaa', > > > and 'xvdba' gives 'xvdaa'/d26p0) > > > > > > > Ok, that's weird. I'll have to figure that out. > > It's probably because 'a' is somtime 0 and sometime is 1. > > 'a' should be 0 > 'aa' should be 26, > 'aaa' Seems to be 702. > > 'xvda': 0 -> 0 * 1 > 'xvdz': 25->25 * 1 > 'xvdaa': 26 ->1 * 26 + 0 * 1 > 'xvdaaa': 702 -> 1 * 26^2 + 1 * 26 + 0 * 1 > > So, it's weird. Have fun fixing the algorithm for that. Actually not as hard as it seemed at first... just treat 'a' as 1 while accumulating the number and then subtract 1 from the result. I wrote a tool to generate the first N disk names using a simple "overflow 'z' to 'aa'" (i.e. non-arithmetical) rule to check against and it matches all the cases I tried (up to 4 letters). Paul > > -- > Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and 'xen-cdrom'
On Fri, Dec 07, 2018 at 02:39:40PM +, Paul Durrant wrote: > > -Original Message- > > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > > Sent: 07 December 2018 14:35 > > To: Paul Durrant > > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; xen- > > de...@lists.xenproject.org; Kevin Wolf ; Max Reitz > > ; Stefano Stabellini > > Subject: Re: [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and > > 'xen-cdrom' > > > > On Thu, Dec 06, 2018 at 03:08:29PM +, Paul Durrant wrote: > > > +static char *disk_to_vbd_name(unsigned int disk) > > > +{ > > > +char *name, *prefix = (disk >= 26) ? > > > +disk_to_vbd_name((disk / 26) - 1) : g_strdup(""); > > > + > > > +name = g_strdup_printf("%s%c", prefix, 'a' + disk); > > > > I don't think that works, if disk is 27, we do ('a' + 27) here. It's > > probably missing a `disk % 26`. > > Damn, yes I was not allowing the >2 letters. > > > > > > +g_free(prefix); > > > + > > > +return name; > > > +} > > > > [...] > > > > > +static unsigned int vbd_name_to_disk(const char *name, const char > > **endp) > > > +{ > > > +unsigned int disk = 0; > > > + > > > +while (*name != '\0') { > > > +if (!g_ascii_isalpha(*name) || !g_ascii_islower(*name)) { > > > +break; > > > +} > > > + > > > +disk *= 26; > > > +disk += *name++ - 'a'; > > > +} > > > +*endp = name; > > > + > > > +return disk; > > > +} > > > + > > > +static void xen_block_set_vdev(Object *obj, Visitor *v, const char > > *name, > > > + void *opaque, Error **errp) > > > +{ > > > > Setting vdev doesn't work. I've tried to add a disk `xvdaa', and it > > result in `xvda', or `d0p0' (in the trace). (Same result with `xvdaaa', > > and 'xvdba' gives 'xvdaa'/d26p0) > > > > Ok, that's weird. I'll have to figure that out. It's probably because 'a' is somtime 0 and sometime is 1. 'a' should be 0 'aa' should be 26, 'aaa' Seems to be 702. 'xvda': 0 -> 0 * 1 'xvdz': 25->25 * 1 'xvdaa': 26 ->1 * 26 + 0 * 1 'xvdaaa': 702 -> 1 * 26^2 + 1 * 26 + 0 * 1 So, it's weird. Have fun fixing the algorithm for that. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and 'xen-cdrom'
> -Original Message- > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > Sent: 07 December 2018 14:35 > To: Paul Durrant > Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; xen- > de...@lists.xenproject.org; Kevin Wolf ; Max Reitz > ; Stefano Stabellini > Subject: Re: [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and > 'xen-cdrom' > > On Thu, Dec 06, 2018 at 03:08:29PM +, Paul Durrant wrote: > > +static char *disk_to_vbd_name(unsigned int disk) > > +{ > > +char *name, *prefix = (disk >= 26) ? > > +disk_to_vbd_name((disk / 26) - 1) : g_strdup(""); > > + > > +name = g_strdup_printf("%s%c", prefix, 'a' + disk); > > I don't think that works, if disk is 27, we do ('a' + 27) here. It's > probably missing a `disk % 26`. Damn, yes I was not allowing the >2 letters. > > > +g_free(prefix); > > + > > +return name; > > +} > > [...] > > > +static unsigned int vbd_name_to_disk(const char *name, const char > **endp) > > +{ > > +unsigned int disk = 0; > > + > > +while (*name != '\0') { > > +if (!g_ascii_isalpha(*name) || !g_ascii_islower(*name)) { > > +break; > > +} > > + > > +disk *= 26; > > +disk += *name++ - 'a'; > > +} > > +*endp = name; > > + > > +return disk; > > +} > > + > > +static void xen_block_set_vdev(Object *obj, Visitor *v, const char > *name, > > + void *opaque, Error **errp) > > +{ > > Setting vdev doesn't work. I've tried to add a disk `xvdaa', and it > result in `xvda', or `d0p0' (in the trace). (Same result with `xvdaaa', > and 'xvdba' gives 'xvdaa'/d26p0) > Ok, that's weird. I'll have to figure that out. Paul > -- > Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and 'xen-cdrom'
On Thu, Dec 06, 2018 at 03:08:29PM +, Paul Durrant wrote: > +static char *disk_to_vbd_name(unsigned int disk) > +{ > +char *name, *prefix = (disk >= 26) ? > +disk_to_vbd_name((disk / 26) - 1) : g_strdup(""); > + > +name = g_strdup_printf("%s%c", prefix, 'a' + disk); I don't think that works, if disk is 27, we do ('a' + 27) here. It's probably missing a `disk % 26`. > +g_free(prefix); > + > +return name; > +} [...] > +static unsigned int vbd_name_to_disk(const char *name, const char **endp) > +{ > +unsigned int disk = 0; > + > +while (*name != '\0') { > +if (!g_ascii_isalpha(*name) || !g_ascii_islower(*name)) { > +break; > +} > + > +disk *= 26; > +disk += *name++ - 'a'; > +} > +*endp = name; > + > +return disk; > +} > + > +static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ Setting vdev doesn't work. I've tried to add a disk `xvdaa', and it result in `xvda', or `d0p0' (in the trace). (Same result with `xvdaaa', and 'xvdba' gives 'xvdaa'/d26p0) -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and 'xen-cdrom'
This patch adds new XenDevice-s: 'xen-disk' and 'xen-cdrom', both derived from a common 'xen-block' parent type. These will eventually replace the 'xen_disk' (note the underscore rather than hyphen) legacy PV backend but it is illustrative to build up the implementation incrementally, along with the XenBus/XenDevice framework. Subsequent patches will therefore add to these devices' implementation as new features are added to the framework. After this patch has been applied it is possible to instantiate new 'xen-disk' or 'xen-cdrom' devices with a single 'vdev' parameter, which accepts values adhering to the Xen VBD naming scheme [1]. For example, a command-line instantiation of a xen-disk can be done with an argument similar to the following: -device xen-disk,vdev=hda The implementation of the vdev parameter formulates the appropriate VBD number for use in the PV protocol. [1] https://xenbits.xen.org/docs/unstable/man/xen-vbd-interface.7.html Signed-off-by: Paul Durrant --- Cc: Kevin Wolf Cc: Max Reitz Cc: Stefano Stabellini Cc: Anthony Perard v2: - Fix boilerplate - Fix vdev parsing - Change name from 'xen-qdisk' to 'xen-block', make abstract, and split off 'xen-disk' and 'xen-cdrom' as concrete sub-types --- MAINTAINERS| 2 +- hw/block/Makefile.objs | 1 + hw/block/trace-events | 8 ++ hw/block/xen-block.c | 347 + include/hw/xen/xen-block.h | 69 + 5 files changed, 426 insertions(+), 1 deletion(-) create mode 100644 hw/block/xen-block.c create mode 100644 include/hw/xen/xen-block.h diff --git a/MAINTAINERS b/MAINTAINERS index 63effdc..dd728c3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -403,7 +403,7 @@ F: hw/9pfs/xen-9p-backend.c F: hw/char/xen_console.c F: hw/display/xenfb.c F: hw/net/xen_nic.c -F: hw/block/xen_* +F: hw/block/xen* F: hw/xen/ F: hw/xenpv/ F: hw/i386/xen/ diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs index 53ce575..f34813a 100644 --- a/hw/block/Makefile.objs +++ b/hw/block/Makefile.objs @@ -4,6 +4,7 @@ common-obj-$(CONFIG_SSI_M25P80) += m25p80.o common-obj-$(CONFIG_NAND) += nand.o common-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o common-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o +common-obj-$(CONFIG_XEN) += xen-block.o common-obj-$(CONFIG_XEN) += xen_disk.o common-obj-$(CONFIG_ECC) += ecc.o common-obj-$(CONFIG_ONENAND) += onenand.o diff --git a/hw/block/trace-events b/hw/block/trace-events index 335c092..4afbd62 100644 --- a/hw/block/trace-events +++ b/hw/block/trace-events @@ -127,3 +127,11 @@ xen_disk_init(char *name) "%s" xen_disk_connect(char *name) "%s" xen_disk_disconnect(char *name) "%s" xen_disk_free(char *name) "%s" + +# hw/block/xen-block.c +xen_block_realize(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u" +xen_block_unrealize(const char *type, uint32_t disk, uint32_t partition) "%s d%up%u" +xen_disk_realize(void) "" +xen_disk_unrealize(void) "" +xen_cdrom_realize(void) "" +xen_cdrom_unrealize(void) "" diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c new file mode 100644 index 000..78f4218 --- /dev/null +++ b/hw/block/xen-block.c @@ -0,0 +1,347 @@ +/* + * Copyright (c) 2018 Citrix Systems Inc. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qemu/cutils.h" +#include "qapi/error.h" +#include "qapi/visitor.h" +#include "hw/hw.h" +#include "hw/xen/xen-block.h" +#include "trace.h" + +static void xen_block_unrealize(XenDevice *xendev, Error **errp) +{ +XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev); +XenBlockDeviceClass *blockdev_class = +XEN_BLOCK_DEVICE_GET_CLASS(xendev); +const char *type = object_get_typename(OBJECT(blockdev)); +XenBlockVdev *vdev = >vdev; +Error *local_err = NULL; + +if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) { +return; +} + +trace_xen_block_unrealize(type, vdev->disk, vdev->partition); + +if (blockdev_class->unrealize) { +blockdev_class->unrealize(blockdev, _err); +if (local_err) { +error_propagate(errp, local_err); +} +} +} + +static void xen_block_realize(XenDevice *xendev, Error **errp) +{ +XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev); +XenBlockDeviceClass *blockdev_class = +XEN_BLOCK_DEVICE_GET_CLASS(xendev); +const char *type = object_get_typename(OBJECT(blockdev)); +XenBlockVdev *vdev = >vdev; +Error *local_err = NULL; + +if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) { +error_setg(errp, "vdev property not set"); +return; +} + +trace_xen_block_realize(type, vdev->disk, vdev->partition); + +if (blockdev_class->realize) { +blockdev_class->realize(blockdev, _err); +if (local_err) { +error_propagate(errp, local_err); +} +} +} + +static char