Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
Am 08.01.2019 um 15:20 hat Paul Durrant geschrieben:
> > -Original Message-
> > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> > Of Paul Durrant
> > Sent: 08 January 2019 14:11
> > To: Anthony Perard
> > Cc: 'Kevin Wolf' ; Stefano Stabellini
> > ; qemu-bl...@nongnu.org; qemu-de...@nongnu.org;
> > Max Reitz ; xen-devel@lists.xenproject.org
> > Subject: Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create
> > XenBlockDevice-s
> >
> > > -Original Message-
> > > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > > Sent: 08 January 2019 13:28
> > > To: Paul Durrant
> > > Cc: 'Kevin Wolf' ; qemu-de...@nongnu.org; qemu-
> > > bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > > ; Stefano Stabellini
> > > Subject: Re: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> > >
> > > On Tue, Jan 08, 2019 at 01:07:49PM +, Paul Durrant wrote:
> > > > > -Original Message-
> > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > Sent: 08 January 2019 12:53
> > > > > To: Paul Durrant
> > > > > Cc: Anthony Perard ; qemu-
> > de...@nongnu.org;
> > > > > qemu-bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > > > > ; Stefano Stabellini
> > > > > Subject: Re: [PATCH v7 16/18] xen: automatically create
> > > XenBlockDevice-s
> > > > >
> > > > > Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben:
> > > > > > > -Original Message-
> > > > > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > > > > > > Sent: 04 January 2019 16:31
> > > > > > > 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 v7 16/18] xen: automatically create
> > > > > XenBlockDevice-s
> > > > > > >
> > > > > > > Almost done, there is one thing left which I believe is an
> > issue.
> > > > > > > Whenever I attach a raw file to QEMU, it print:
> > > > > > > qemu-system-i386: warning: Opening a block device as a file
> > > using
> > > > > the
> > > > > > > 'file' driver is deprecated
> > > > > >
> > > > > > Oh, I'd not noticed that... but then I only use raw files
> > > occasionally.
> > > > >
> > > > > Strictly speaking, this is not about raw (regular) files, but raw
> > > block
> > > > > devices. 'file' is fine for actual regular files, but the protocol
> > > > > driver for block devices is 'host_device'.
> > > > >
> > > > > > > raw files should use the "raw" driver, so we aren't done yet.
> > > > > >
> > > > > > Ok. Having a strictly 2-layer stack actually makes things simpler
> > > anyway
> > > > > :-)
> > > > >
> > > > > Using 'raw' there will make the block layer auto-detect the right
> > > > > protocol layer, so this works. If you want to avoid the second
> > layer,
> > > > > you'd have to figure out manually whether to use 'file' or
> > > > > 'host_device'.
> > > >
> > > > Thanks for the explanation. I'll give it a spin using a device... I've
> > > posted v8 but, given what you say, I'm still not sure I have it right.
> > >
> > > Indeed, in v8, even with the extra 'raw' layer, the warning is still
> > > there. I was trying to understand why, and I only found out today that
> > > we would need to use the 'host_device' driver as explain by Kevin.
> > >
> > >
> > > BTW Paul, we can simplify the code that manage layers, by not managing
> > > them.
> > > Instead of (in JSON / QMP term):
> > > { 'driver': 'file', 'filename': '/file', 'node-name': 'node-file' }
> > > { 'driver': 'qcow2', 'file': 'node-file', 'node-name': 'node-qcow2'
> > }
> > > we can have:
> > > { 'driver': 'qcow2', 'node-name': 'node-qcow2',
> > > 'file': { 'driver': 'file', 'filename': '/file' } }
> > >
> >
> > I kind of like the clean separation though... From what Kevin said, it
> > sounds like the lowest layer should use 'raw' instead of 'file' to DTRT,
> > and then we should be back to only needing the single layer in that case.
> > I'll revert back to v7 and give it a try.
>
> No, that doesn't work as we can't deal with locking correctly unless
> we use driver=file so maybe I misunderstood.
The lowest layer should use 'host_device' there. 'raw' is the format
layer that is stacked on top.
I'm not sure what your locking requirements are, but 'host_device'
supports the same options as 'file'.
Kevin
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> I've tested your patch and it does seem like the best way forward. I'll > squash it in. Do you want me to put your S-o-b on the combined patch? You can, I'll have to add it anyway when I'll prepare the pull request. Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 08 January 2019 13:28
> To: Paul Durrant
> Cc: 'Kevin Wolf' ; qemu-de...@nongnu.org; qemu-
> bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> ; Stefano Stabellini
> Subject: Re: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
>
> On Tue, Jan 08, 2019 at 01:07:49PM +, Paul Durrant wrote:
> > > -Original Message-
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Sent: 08 January 2019 12:53
> > > To: Paul Durrant
> > > Cc: Anthony Perard ; qemu-de...@nongnu.org;
> > > qemu-bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > > ; Stefano Stabellini
> > > Subject: Re: [PATCH v7 16/18] xen: automatically create
> XenBlockDevice-s
> > >
> > > Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben:
> > > > > -Original Message-
> > > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > > > > Sent: 04 January 2019 16:31
> > > > > 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 v7 16/18] xen: automatically create
> > > XenBlockDevice-s
> > > > >
> > > > > Almost done, there is one thing left which I believe is an issue.
> > > > > Whenever I attach a raw file to QEMU, it print:
> > > > > qemu-system-i386: warning: Opening a block device as a file
> using
> > > the
> > > > > 'file' driver is deprecated
> > > >
> > > > Oh, I'd not noticed that... but then I only use raw files
> occasionally.
> > >
> > > Strictly speaking, this is not about raw (regular) files, but raw
> block
> > > devices. 'file' is fine for actual regular files, but the protocol
> > > driver for block devices is 'host_device'.
> > >
> > > > > raw files should use the "raw" driver, so we aren't done yet.
> > > >
> > > > Ok. Having a strictly 2-layer stack actually makes things simpler
> anyway
> > > :-)
> > >
> > > Using 'raw' there will make the block layer auto-detect the right
> > > protocol layer, so this works. If you want to avoid the second layer,
> > > you'd have to figure out manually whether to use 'file' or
> > > 'host_device'.
> >
> > Thanks for the explanation. I'll give it a spin using a device... I've
> posted v8 but, given what you say, I'm still not sure I have it right.
>
> Indeed, in v8, even with the extra 'raw' layer, the warning is still
> there. I was trying to understand why, and I only found out today that
> we would need to use the 'host_device' driver as explain by Kevin.
>
>
> BTW Paul, we can simplify the code that manage layers, by not managing
> them.
> Instead of (in JSON / QMP term):
> { 'driver': 'file', 'filename': '/file', 'node-name': 'node-file' }
> { 'driver': 'qcow2', 'file': 'node-file', 'node-name': 'node-qcow2' }
> we can have:
> { 'driver': 'qcow2', 'node-name': 'node-qcow2',
> 'file': { 'driver': 'file', 'filename': '/file' } }
>
>
> Here is the patch I have, fill free to review and squash it, or not:
I've tested your patch and it does seem like the best way forward. I'll squash
it in. Do you want me to put your S-o-b on the combined patch?
Paul
>
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index 91f5b58993..c6ec1d9543 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -653,43 +653,23 @@ static char *xen_block_blockdev_add(const char *id,
> QDict *qdict,
>
> static void xen_block_drive_destroy(XenBlockDrive *drive, Error **errp)
> {
> -while (drive->layers-- != 0) {
> -char *node_name = drive->node_name[drive->layers];
> +char *node_name = drive->node_name;
> +
> +if (node_name) {
> Error *local_err = NULL;
>
> xen_block_blockdev_del(node_name, _err);
> if (local_err) {
> error_propagate(errp, local_err);
> -drive->layers++;
> return;
> }
> +g_free(node_name);
> +drive->node_name = NULL;
> }
> g_free(drive->id);
> g_free(drive);
> }
>
> -static void xen_block_drive_layer_add(XenBlockDrive *drive, QDict *qdict,
> - Error **errp)
> -{
> -unsigned int i = drive->layers;
> -char *node_name;
> -
> -g_assert(drive->layers < ARRAY_SIZE(drive->node_name));
> -
> -if (i != 0) {
> -/* Link to the lower layer */
> -qdict_put_str(qdict, "file", drive->node_name[i - 1]);
> -}
> -
> -node_name = xen_block_blockdev_add(drive->id, qdict, errp);
> -if (!node_name) {
> -return;
> -}
> -
> -drive->node_name[i] = node_name;
> -drive->layers++;
> -}
> -
> static XenBlockDrive *xen_block_drive_create(const char *id,
> const char *device_type,
> QDict *opts, Error **errp)
> @@ -702,7 +682,8 @@ static XenBlockDrive
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 08 January 2019 14:11
> To: Anthony Perard
> Cc: 'Kevin Wolf' ; Stefano Stabellini
> ; qemu-bl...@nongnu.org; qemu-de...@nongnu.org;
> Max Reitz ; xen-devel@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create
> XenBlockDevice-s
>
> > -Original Message-
> > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > Sent: 08 January 2019 13:28
> > To: Paul Durrant
> > Cc: 'Kevin Wolf' ; qemu-de...@nongnu.org; qemu-
> > bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > ; Stefano Stabellini
> > Subject: Re: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> >
> > On Tue, Jan 08, 2019 at 01:07:49PM +, Paul Durrant wrote:
> > > > -Original Message-
> > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > Sent: 08 January 2019 12:53
> > > > To: Paul Durrant
> > > > Cc: Anthony Perard ; qemu-
> de...@nongnu.org;
> > > > qemu-bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > > > ; Stefano Stabellini
> > > > Subject: Re: [PATCH v7 16/18] xen: automatically create
> > XenBlockDevice-s
> > > >
> > > > Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben:
> > > > > > -Original Message-
> > > > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > > > > > Sent: 04 January 2019 16:31
> > > > > > 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 v7 16/18] xen: automatically create
> > > > XenBlockDevice-s
> > > > > >
> > > > > > Almost done, there is one thing left which I believe is an
> issue.
> > > > > > Whenever I attach a raw file to QEMU, it print:
> > > > > > qemu-system-i386: warning: Opening a block device as a file
> > using
> > > > the
> > > > > > 'file' driver is deprecated
> > > > >
> > > > > Oh, I'd not noticed that... but then I only use raw files
> > occasionally.
> > > >
> > > > Strictly speaking, this is not about raw (regular) files, but raw
> > block
> > > > devices. 'file' is fine for actual regular files, but the protocol
> > > > driver for block devices is 'host_device'.
> > > >
> > > > > > raw files should use the "raw" driver, so we aren't done yet.
> > > > >
> > > > > Ok. Having a strictly 2-layer stack actually makes things simpler
> > anyway
> > > > :-)
> > > >
> > > > Using 'raw' there will make the block layer auto-detect the right
> > > > protocol layer, so this works. If you want to avoid the second
> layer,
> > > > you'd have to figure out manually whether to use 'file' or
> > > > 'host_device'.
> > >
> > > Thanks for the explanation. I'll give it a spin using a device... I've
> > posted v8 but, given what you say, I'm still not sure I have it right.
> >
> > Indeed, in v8, even with the extra 'raw' layer, the warning is still
> > there. I was trying to understand why, and I only found out today that
> > we would need to use the 'host_device' driver as explain by Kevin.
> >
> >
> > BTW Paul, we can simplify the code that manage layers, by not managing
> > them.
> > Instead of (in JSON / QMP term):
> > { 'driver': 'file', 'filename': '/file', 'node-name': 'node-file' }
> > { 'driver': 'qcow2', 'file': 'node-file', 'node-name': 'node-qcow2'
> }
> > we can have:
> > { 'driver': 'qcow2', 'node-name': 'node-qcow2',
> > 'file': { 'driver': 'file', 'filename': '/file' } }
> >
>
> I kind of like the clean separation though... From what Kevin said, it
> sounds like the lowest layer should use 'raw' instead of 'file' to DTRT,
> and then we should be back to only needing the single layer in that case.
> I'll revert back to v7 and give it a try.
No, that doesn't work as we can't deal with locking correctly unless we use
driver=file so maybe I misunderstood.
Paul
>
> Paul
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 08 January 2019 13:28
> To: Paul Durrant
> Cc: 'Kevin Wolf' ; qemu-de...@nongnu.org; qemu-
> bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> ; Stefano Stabellini
> Subject: Re: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
>
> On Tue, Jan 08, 2019 at 01:07:49PM +, Paul Durrant wrote:
> > > -Original Message-
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Sent: 08 January 2019 12:53
> > > To: Paul Durrant
> > > Cc: Anthony Perard ; qemu-de...@nongnu.org;
> > > qemu-bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > > ; Stefano Stabellini
> > > Subject: Re: [PATCH v7 16/18] xen: automatically create
> XenBlockDevice-s
> > >
> > > Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben:
> > > > > -Original Message-
> > > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > > > > Sent: 04 January 2019 16:31
> > > > > 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 v7 16/18] xen: automatically create
> > > XenBlockDevice-s
> > > > >
> > > > > Almost done, there is one thing left which I believe is an issue.
> > > > > Whenever I attach a raw file to QEMU, it print:
> > > > > qemu-system-i386: warning: Opening a block device as a file
> using
> > > the
> > > > > 'file' driver is deprecated
> > > >
> > > > Oh, I'd not noticed that... but then I only use raw files
> occasionally.
> > >
> > > Strictly speaking, this is not about raw (regular) files, but raw
> block
> > > devices. 'file' is fine for actual regular files, but the protocol
> > > driver for block devices is 'host_device'.
> > >
> > > > > raw files should use the "raw" driver, so we aren't done yet.
> > > >
> > > > Ok. Having a strictly 2-layer stack actually makes things simpler
> anyway
> > > :-)
> > >
> > > Using 'raw' there will make the block layer auto-detect the right
> > > protocol layer, so this works. If you want to avoid the second layer,
> > > you'd have to figure out manually whether to use 'file' or
> > > 'host_device'.
> >
> > Thanks for the explanation. I'll give it a spin using a device... I've
> posted v8 but, given what you say, I'm still not sure I have it right.
>
> Indeed, in v8, even with the extra 'raw' layer, the warning is still
> there. I was trying to understand why, and I only found out today that
> we would need to use the 'host_device' driver as explain by Kevin.
>
>
> BTW Paul, we can simplify the code that manage layers, by not managing
> them.
> Instead of (in JSON / QMP term):
> { 'driver': 'file', 'filename': '/file', 'node-name': 'node-file' }
> { 'driver': 'qcow2', 'file': 'node-file', 'node-name': 'node-qcow2' }
> we can have:
> { 'driver': 'qcow2', 'node-name': 'node-qcow2',
> 'file': { 'driver': 'file', 'filename': '/file' } }
>
I kind of like the clean separation though... From what Kevin said, it sounds
like the lowest layer should use 'raw' instead of 'file' to DTRT, and then we
should be back to only needing the single layer in that case. I'll revert back
to v7 and give it a try.
Paul
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
On Tue, Jan 08, 2019 at 01:07:49PM +, Paul Durrant wrote:
> > -Original Message-
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Sent: 08 January 2019 12:53
> > To: Paul Durrant
> > Cc: Anthony Perard ; qemu-de...@nongnu.org;
> > qemu-bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz
> > ; Stefano Stabellini
> > Subject: Re: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> >
> > Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben:
> > > > -Original Message-
> > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> > > > Sent: 04 January 2019 16:31
> > > > 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 v7 16/18] xen: automatically create
> > XenBlockDevice-s
> > > >
> > > > Almost done, there is one thing left which I believe is an issue.
> > > > Whenever I attach a raw file to QEMU, it print:
> > > > qemu-system-i386: warning: Opening a block device as a file using
> > the
> > > > 'file' driver is deprecated
> > >
> > > Oh, I'd not noticed that... but then I only use raw files occasionally.
> >
> > Strictly speaking, this is not about raw (regular) files, but raw block
> > devices. 'file' is fine for actual regular files, but the protocol
> > driver for block devices is 'host_device'.
> >
> > > > raw files should use the "raw" driver, so we aren't done yet.
> > >
> > > Ok. Having a strictly 2-layer stack actually makes things simpler anyway
> > :-)
> >
> > Using 'raw' there will make the block layer auto-detect the right
> > protocol layer, so this works. If you want to avoid the second layer,
> > you'd have to figure out manually whether to use 'file' or
> > 'host_device'.
>
> Thanks for the explanation. I'll give it a spin using a device... I've posted
> v8 but, given what you say, I'm still not sure I have it right.
Indeed, in v8, even with the extra 'raw' layer, the warning is still
there. I was trying to understand why, and I only found out today that
we would need to use the 'host_device' driver as explain by Kevin.
BTW Paul, we can simplify the code that manage layers, by not managing
them.
Instead of (in JSON / QMP term):
{ 'driver': 'file', 'filename': '/file', 'node-name': 'node-file' }
{ 'driver': 'qcow2', 'file': 'node-file', 'node-name': 'node-qcow2' }
we can have:
{ 'driver': 'qcow2', 'node-name': 'node-qcow2',
'file': { 'driver': 'file', 'filename': '/file' } }
Here is the patch I have, fill free to review and squash it, or not:
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 91f5b58993..c6ec1d9543 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -653,43 +653,23 @@ static char *xen_block_blockdev_add(const char *id, QDict
*qdict,
static void xen_block_drive_destroy(XenBlockDrive *drive, Error **errp)
{
-while (drive->layers-- != 0) {
-char *node_name = drive->node_name[drive->layers];
+char *node_name = drive->node_name;
+
+if (node_name) {
Error *local_err = NULL;
xen_block_blockdev_del(node_name, _err);
if (local_err) {
error_propagate(errp, local_err);
-drive->layers++;
return;
}
+g_free(node_name);
+drive->node_name = NULL;
}
g_free(drive->id);
g_free(drive);
}
-static void xen_block_drive_layer_add(XenBlockDrive *drive, QDict *qdict,
- Error **errp)
-{
-unsigned int i = drive->layers;
-char *node_name;
-
-g_assert(drive->layers < ARRAY_SIZE(drive->node_name));
-
-if (i != 0) {
-/* Link to the lower layer */
-qdict_put_str(qdict, "file", drive->node_name[i - 1]);
-}
-
-node_name = xen_block_blockdev_add(drive->id, qdict, errp);
-if (!node_name) {
-return;
-}
-
-drive->node_name[i] = node_name;
-drive->layers++;
-}
-
static XenBlockDrive *xen_block_drive_create(const char *id,
const char *device_type,
QDict *opts, Error **errp)
@@ -702,7 +682,8 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
char *filename = NULL;
XenBlockDrive *drive = NULL;
Error *local_err = NULL;
-QDict *qdict;
+QDict *file_layer;
+QDict *driver_layer;
if (params) {
char **v = g_strsplit(params, ":", 2);
@@ -733,13 +714,13 @@ static XenBlockDrive *xen_block_drive_create(const char
*id,
drive = g_new0(XenBlockDrive, 1);
drive->id = g_strdup(id);
-qdict = qdict_new();
+file_layer = qdict_new();
-qdict_put_str(qdict, "driver", "file");
-qdict_put_str(qdict, "filename", filename);
+qdict_put_str(file_layer, "driver", "file");
+qdict_put_str(file_layer, "filename", filename);
if (mode && *mode != 'w') {
-
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> -Original Message- > From: Kevin Wolf [mailto:kw...@redhat.com] > Sent: 08 January 2019 12:53 > To: Paul Durrant > Cc: Anthony Perard ; qemu-de...@nongnu.org; > qemu-bl...@nongnu.org; xen-devel@lists.xenproject.org; Max Reitz > ; Stefano Stabellini > Subject: Re: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s > > Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben: > > > -Original Message- > > > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > > > Sent: 04 January 2019 16:31 > > > 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 v7 16/18] xen: automatically create > XenBlockDevice-s > > > > > > Almost done, there is one thing left which I believe is an issue. > > > Whenever I attach a raw file to QEMU, it print: > > > qemu-system-i386: warning: Opening a block device as a file using > the > > > 'file' driver is deprecated > > > > Oh, I'd not noticed that... but then I only use raw files occasionally. > > Strictly speaking, this is not about raw (regular) files, but raw block > devices. 'file' is fine for actual regular files, but the protocol > driver for block devices is 'host_device'. > > > > raw files should use the "raw" driver, so we aren't done yet. > > > > Ok. Having a strictly 2-layer stack actually makes things simpler anyway > :-) > > Using 'raw' there will make the block layer auto-detect the right > protocol layer, so this works. If you want to avoid the second layer, > you'd have to figure out manually whether to use 'file' or > 'host_device'. Thanks for the explanation. I'll give it a spin using a device... I've posted v8 but, given what you say, I'm still not sure I have it right. Paul > > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
Am 04.01.2019 um 17:40 hat Paul Durrant geschrieben: > > -Original Message- > > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > > Sent: 04 January 2019 16:31 > > 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 v7 16/18] xen: automatically create XenBlockDevice-s > > > > Almost done, there is one thing left which I believe is an issue. > > Whenever I attach a raw file to QEMU, it print: > > qemu-system-i386: warning: Opening a block device as a file using the > > 'file' driver is deprecated > > Oh, I'd not noticed that... but then I only use raw files occasionally. Strictly speaking, this is not about raw (regular) files, but raw block devices. 'file' is fine for actual regular files, but the protocol driver for block devices is 'host_device'. > > raw files should use the "raw" driver, so we aren't done yet. > > Ok. Having a strictly 2-layer stack actually makes things simpler anyway :-) Using 'raw' there will make the block layer auto-detect the right protocol layer, so this works. If you want to avoid the second layer, you'd have to figure out manually whether to use 'file' or 'host_device'. Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 04 January 2019 16:31
> 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 v7 16/18] xen: automatically create XenBlockDevice-s
>
> Almost done, there is one thing left which I believe is an issue.
> Whenever I attach a raw file to QEMU, it print:
> qemu-system-i386: warning: Opening a block device as a file using the
> 'file' driver is deprecated
Oh, I'd not noticed that... but then I only use raw files occasionally.
>
> So, I think the comment below isn't true. We should create a "raw"
> driver for "raw" files.
>
> On Thu, Dec 20, 2018 at 05:14:37PM +, Paul Durrant wrote:
> > +static XenBlockDrive *xen_block_drive_create(const char *id,
> > + const char *device_type,
> > + QDict *opts, Error **errp)
> > +{
> ...
>
> > +if (params) {
> > +char **v = g_strsplit(params, ":", 2);
> > +
> > +if (v[1] == NULL) {
> > +filename = g_strdup(v[0]);
> > +driver = g_strdup("file");
> > +} else {
> > +if (strcmp(v[0], "aio") == 0) {
> > +driver = g_strdup("file");
> > +} else if (strcmp(v[0], "vhd") == 0) {
> > +driver = g_strdup("vpc");
> > +} else {
> > +driver = g_strdup(v[0]);
> > +}
> > +filename = g_strdup(v[1]);
> > +}
> > +
> > +g_strfreev(v);
> > +}
> > +
> ...
>
> > +/* If the image is a raw file then we are done */
>
> raw files should use the "raw" driver, so we aren't done yet.
>
Ok. Having a strictly 2-layer stack actually makes things simpler anyway :-)
Paul
> > +if (!strcmp(driver, "file")) {
> > +goto done;
> > +}
>
> --
> Anthony PERARD
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
Almost done, there is one thing left which I believe is an issue.
Whenever I attach a raw file to QEMU, it print:
qemu-system-i386: warning: Opening a block device as a file using the
'file' driver is deprecated
So, I think the comment below isn't true. We should create a "raw"
driver for "raw" files.
On Thu, Dec 20, 2018 at 05:14:37PM +, Paul Durrant wrote:
> +static XenBlockDrive *xen_block_drive_create(const char *id,
> + const char *device_type,
> + QDict *opts, Error **errp)
> +{
...
> +if (params) {
> +char **v = g_strsplit(params, ":", 2);
> +
> +if (v[1] == NULL) {
> +filename = g_strdup(v[0]);
> +driver = g_strdup("file");
> +} else {
> +if (strcmp(v[0], "aio") == 0) {
> +driver = g_strdup("file");
> +} else if (strcmp(v[0], "vhd") == 0) {
> +driver = g_strdup("vpc");
> +} else {
> +driver = g_strdup(v[0]);
> +}
> +filename = g_strdup(v[1]);
> +}
> +
> +g_strfreev(v);
> +}
> +
...
> +/* If the image is a raw file then we are done */
raw files should use the "raw" driver, so we aren't done yet.
> +if (!strcmp(driver, "file")) {
> +goto done;
> +}
--
Anthony PERARD
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
Ping Anthony & Kevin?
I believe this version is purged of all legacy interface use.
> -Original Message-
> From: Paul Durrant [mailto:paul.durr...@citrix.com]
> Sent: 20 December 2018 17:15
> To: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; xen-
> de...@lists.xenproject.org
> Cc: Paul Durrant ; Anthony Perard
> ; Kevin Wolf ; Max Reitz
> ; Stefano Stabellini
> Subject: [PATCH v7 16/18] xen: automatically create XenBlockDevice-s
>
> This patch adds create and destroy function for XenBlockDevice-s so that
> they can be created automatically when the Xen toolstack instantiates a
> new
> PV backend via xenstore. When the XenBlockDevice is created this way it is
> also necessary to create a 'drive' which matches the configuration that
> the
> Xen toolstack has written into xenstore. This is done by formulating the
> parameters necessary for each 'blockdev' layer of the drive and then using
> qmp_blockdev_add() to create the layers. Also, for compatibility with the
> legacy 'xen_disk' implementation, an iothread is automatically created for
> the new XenBlockDevice. This, like the driver layers, will be destroyed
> after the XenBlockDevice is unrealized.
>
> The legacy backend scan for 'qdisk' is removed by this patch, which makes
> the 'xen_disk' code is redundant. The code will be removed by a subsequent
> patch.
>
> Signed-off-by: Paul Durrant
> ---
> Cc: Anthony Perard
> Cc: Kevin Wolf
> Cc: Max Reitz
> Cc: Stefano Stabellini
>
> v7:
> - Don't use qobject_input_visitor_new_flat_confused()
>
> v5:
> - Extensively re-worked to avoid using drive_new() and use
>qmp_blockdev_add() instead
> - Also use qmp_object_add() for IOThread
> - Dropped Anthony's R-b because of the code changes
>
> v2:
> - Get rid of error_abort
> - Don't use qdev_init_nofail()
> - Explain why file locking needs to be off
> ---
> hw/block/trace-events | 4 +
> hw/block/xen-block.c| 404
> hw/xen/xen-legacy-backend.c | 1 -
> include/hw/xen/xen-block.h | 13 ++
> 4 files changed, 421 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 89e258319c..55e5a5500c 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -137,3 +137,7 @@ xen_disk_realize(void) ""
> xen_disk_unrealize(void) ""
> xen_cdrom_realize(void) ""
> xen_cdrom_unrealize(void) ""
> +xen_block_blockdev_add(char *str) "%s"
> +xen_block_blockdev_del(const char *node_name) "%s"
> +xen_block_device_create(unsigned int number) "%u"
> +xen_block_device_destroy(unsigned int number) "%u"
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index a7c37c185a..1e34fe1527 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -7,12 +7,20 @@
>
> #include "qemu/osdep.h"
> #include "qemu/cutils.h"
> +#include "qemu/option.h"
> #include "qapi/error.h"
> +#include "qapi/qapi-commands-block-core.h"
> +#include "qapi/qapi-commands-misc.h"
> +#include "qapi/qapi-visit-block-core.h"
> +#include "qapi/qobject-input-visitor.h"
> #include "qapi/visitor.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qstring.h"
> #include "hw/hw.h"
> #include "hw/xen/xen_common.h"
> #include "hw/block/xen_blkif.h"
> #include "hw/xen/xen-block.h"
> +#include "hw/xen/xen-backend.h"
> #include "sysemu/blockdev.h"
> #include "sysemu/block-backend.h"
> #include "sysemu/iothread.h"
> @@ -474,6 +482,7 @@ static void xen_block_class_init(ObjectClass *class,
> void *data)
> DeviceClass *dev_class = DEVICE_CLASS(class);
> XenDeviceClass *xendev_class = XEN_DEVICE_CLASS(class);
>
> +xendev_class->backend = "qdisk";
> xendev_class->device = "vbd";
> xendev_class->get_name = xen_block_get_name;
> xendev_class->realize = xen_block_realize;
> @@ -586,3 +595,398 @@ static void xen_block_register_types(void)
> }
>
> type_init(xen_block_register_types)
> +
> +static void xen_block_blockdev_del(const char *node_name, Error **errp)
> +{
> +trace_xen_block_blockdev_del(node_name);
> +
> +qmp_blockdev_del(node_name, errp);
> +}
> +
> +static char *xen_block_blockdev_add(const char *id, QDict *qdict,
> +Error **errp)
> +{
> +const char *driver = qdict_get_try_str(qdict, "driver");
> +BlockdevOptions *options = NULL;
> +Error *local_err = NULL;
> +char *node_name;
> +Visitor *v;
> +
> +if (!driver) {
> +error_setg(errp, "no 'driver' parameter");
> +return NULL;
> +}
> +
> +node_name = g_strdup_printf("%s-%s", id, driver);
> +qdict_put_str(qdict, "node-name", node_name);
> +
> +trace_xen_block_blockdev_add(node_name);
> +
> +v = qobject_input_visitor_new(QOBJECT(qdict));
> +visit_type_BlockdevOptions(v, NULL, , _err);
> +visit_free(v);
> +
> +if (local_err) {
> +error_propagate(errp, local_err);
> +goto fail;
> +}
> +
> +qmp_blockdev_add(options, _err);
> +
> +if
