| Message ID | 20200813162935.210070-10-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/export: Add infrastructure and QAPI for block exports | expand |
On 13.08.20 18:29, Kevin Wolf wrote: > qemu-nbd allows use of writethrough cache modes, which mean that write > requests made through NBD will cause a flush before they complete. > Expose the same functionality in block-export-add. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/block-export.json | 7 ++++++- > blockdev-nbd.c | 2 +- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/qapi/block-export.json b/qapi/block-export.json > index 1fdc55c53a..4ce163411f 100644 > --- a/qapi/block-export.json > +++ b/qapi/block-export.json > @@ -167,10 +167,15 @@ > # Describes a block export, i.e. how single node should be exported on an > # external interface. > # > +# @writethrough: If true, caches are flushed after every write request to the > +# export before completion is signalled. (since: 5.2; > +# default: false) > +# > # Since: 4.2 > ## > { 'union': 'BlockExportOptions', > - 'base': { 'type': 'BlockExportType' }, > + 'base': { 'type': 'BlockExportType', > + '*writethrough': 'bool' }, > 'discriminator': 'type', > 'data': { > 'nbd': 'BlockExportOptionsNbd' Hm. I find it weird to have @writethrough in the base but @device in the specialized class. I think everything that will be common to all block exports should be in the base, and that probably includes a node-name. I’m aware that will make things more tedious in the code, but perhaps it would be a nicer interface in the end. Or is the real problem that that would create problems in the storage daemon’s command line interface, because then the specialized (legacy) NBD interface would no longer be compatible with the new generalized block export interface? Anyway, @writable might be a similar story. A @read-only may make sense in general, I think. Basically, I think that the export code should be separate from the code setting up the BlockBackend that should be exported, so all options regarding that BB should be common; and those options are @node-name, @writethrough, and @read-only. (And perhaps other things like @resizable, too, even though that isn’t something to consider for NBD.) Max
Am 17.08.2020 um 14:56 hat Max Reitz geschrieben: > On 13.08.20 18:29, Kevin Wolf wrote: > > qemu-nbd allows use of writethrough cache modes, which mean that write > > requests made through NBD will cause a flush before they complete. > > Expose the same functionality in block-export-add. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > qapi/block-export.json | 7 ++++++- > > blockdev-nbd.c | 2 +- > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/qapi/block-export.json b/qapi/block-export.json > > index 1fdc55c53a..4ce163411f 100644 > > --- a/qapi/block-export.json > > +++ b/qapi/block-export.json > > @@ -167,10 +167,15 @@ > > # Describes a block export, i.e. how single node should be exported on an > > # external interface. > > # > > +# @writethrough: If true, caches are flushed after every write request to the > > +# export before completion is signalled. (since: 5.2; > > +# default: false) > > +# > > # Since: 4.2 > > ## > > { 'union': 'BlockExportOptions', > > - 'base': { 'type': 'BlockExportType' }, > > + 'base': { 'type': 'BlockExportType', > > + '*writethrough': 'bool' }, > > 'discriminator': 'type', > > 'data': { > > 'nbd': 'BlockExportOptionsNbd' > > Hm. I find it weird to have @writethrough in the base but @device in > the specialized class. > > I think everything that will be common to all block exports should be in > the base, and that probably includes a node-name. I’m aware that will > make things more tedious in the code, but perhaps it would be a nicer > interface in the end. Or is the real problem that that would create > problems in the storage daemon’s command line interface, because then > the specialized (legacy) NBD interface would no longer be compatible > with the new generalized block export interface? Indeed. I think patch 15 has what you're looking for. > Anyway, @writable might be a similar story. A @read-only may make sense > in general, I think. Pulling @writable up is easier than a @read-only, but that's a naming detail. In general I agree, but this part isn't addressed in this series yet. Part of the reason why this is an RFC was to find out if I should include things like this or if we'll do it when we add another export type or common functionality that needs the same option. > Basically, I think that the export code should be separate from the code > setting up the BlockBackend that should be exported, so all options > regarding that BB should be common; and those options are @node-name, > @writethrough, and @read-only. (And perhaps other things like > @resizable, too, even though that isn’t something to consider for NBD.) Do you mean that the BlockBackend should already be created by the generic block export layer? Kevin
On 17.08.20 15:13, Kevin Wolf wrote: > Am 17.08.2020 um 14:56 hat Max Reitz geschrieben: >> On 13.08.20 18:29, Kevin Wolf wrote: >>> qemu-nbd allows use of writethrough cache modes, which mean that write >>> requests made through NBD will cause a flush before they complete. >>> Expose the same functionality in block-export-add. >>> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> qapi/block-export.json | 7 ++++++- >>> blockdev-nbd.c | 2 +- >>> 2 files changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/qapi/block-export.json b/qapi/block-export.json >>> index 1fdc55c53a..4ce163411f 100644 >>> --- a/qapi/block-export.json >>> +++ b/qapi/block-export.json >>> @@ -167,10 +167,15 @@ >>> # Describes a block export, i.e. how single node should be exported on an >>> # external interface. >>> # >>> +# @writethrough: If true, caches are flushed after every write request to the >>> +# export before completion is signalled. (since: 5.2; >>> +# default: false) >>> +# >>> # Since: 4.2 >>> ## >>> { 'union': 'BlockExportOptions', >>> - 'base': { 'type': 'BlockExportType' }, >>> + 'base': { 'type': 'BlockExportType', >>> + '*writethrough': 'bool' }, >>> 'discriminator': 'type', >>> 'data': { >>> 'nbd': 'BlockExportOptionsNbd' >> >> Hm. I find it weird to have @writethrough in the base but @device in >> the specialized class. >> >> I think everything that will be common to all block exports should be in >> the base, and that probably includes a node-name. I’m aware that will >> make things more tedious in the code, but perhaps it would be a nicer >> interface in the end. Or is the real problem that that would create >> problems in the storage daemon’s command line interface, because then >> the specialized (legacy) NBD interface would no longer be compatible >> with the new generalized block export interface? > > Indeed. I think patch 15 has what you're looking for. Great. :) Discussions where both participants have the same opinion from the start are the best ones. >> Anyway, @writable might be a similar story. A @read-only may make sense >> in general, I think. > > Pulling @writable up is easier than a @read-only, but that's a naming > detail. Sure. > In general I agree, but this part isn't addressed in this series yet. > Part of the reason why this is an RFC was to find out if I should > include things like this or if we'll do it when we add another export > type or common functionality that needs the same option. Sure, sure. Meta: I personally don’t like RFC patches very much. RFC to me means everything is fair game, and reviewers should be free to let their thoughts wander and come up with perhaps wild ideas, just trying to gauge what everyone thinks. When I’m the submitter, I tend to get defensive then, because I’ve invested time in writing the code already, so I tend to be biased against fundamental changes. (Horrible personal trait. I’m working on it.) As a reviewer, the code and thus some fleshed out design is there already, so it’s difficult to break free from that and find completely different solutions to the original problem. (I kind of ventured in that direction for this patch, and it seems like you immediately noticed that my response was different from usual and pointed out the RFC status, perhaps to make me feel more comfortable in questioning the fundamental design more. Which I noticed, hence this wall of text.) Perhaps I’m wrong. Perhaps it’s just myself (the points I’ve just listed are definitely my own personal weaknesses), but I can’t help but project and assume that others may feel similar, at least in part. So I feel like RFCs that consist of patches tend to at least lock me in to the solution that’s present. I find them difficult to handle, both as a submitter and as a reviewer. All in all, that means on either side I tend to handle patch RFCs as “Standard series, just tests missing”. Not sure if that’s ideal. Or maybe that’s exactly what patch RFCs are? (Of course, it can and should be argued that even for standard series, I shouldn’t be afraid of questioning the fundamental design still. But that’s hard...) But, well. The alternative is writing pure design RFCs, and then you tend to get weeks of slow discussion, drawing everything out. Which isn’t ideal either. Or is that just a baseless prejudice I have? >> Basically, I think that the export code should be separate from the code >> setting up the BlockBackend that should be exported, so all options >> regarding that BB should be common; and those options are @node-name, >> @writethrough, and @read-only. (And perhaps other things like >> @resizable, too, even though that isn’t something to consider for NBD.) > > Do you mean that the BlockBackend should already be created by the > generic block export layer? It would certainly be nice, if it were feasible, don’t you think? We don’t have to bend backwards for it, but maybe it would force us to bring the natural separation of block device and export parameters to the interface. Max
Am 17.08.2020 um 15:51 hat Max Reitz geschrieben: > On 17.08.20 15:13, Kevin Wolf wrote: > > Am 17.08.2020 um 14:56 hat Max Reitz geschrieben: > >> On 13.08.20 18:29, Kevin Wolf wrote: > >>> qemu-nbd allows use of writethrough cache modes, which mean that write > >>> requests made through NBD will cause a flush before they complete. > >>> Expose the same functionality in block-export-add. > >>> > >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> > >>> --- > >>> qapi/block-export.json | 7 ++++++- > >>> blockdev-nbd.c | 2 +- > >>> 2 files changed, 7 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/qapi/block-export.json b/qapi/block-export.json > >>> index 1fdc55c53a..4ce163411f 100644 > >>> --- a/qapi/block-export.json > >>> +++ b/qapi/block-export.json > >>> @@ -167,10 +167,15 @@ > >>> # Describes a block export, i.e. how single node should be exported on an > >>> # external interface. > >>> # > >>> +# @writethrough: If true, caches are flushed after every write request to the > >>> +# export before completion is signalled. (since: 5.2; > >>> +# default: false) > >>> +# > >>> # Since: 4.2 > >>> ## > >>> { 'union': 'BlockExportOptions', > >>> - 'base': { 'type': 'BlockExportType' }, > >>> + 'base': { 'type': 'BlockExportType', > >>> + '*writethrough': 'bool' }, > >>> 'discriminator': 'type', > >>> 'data': { > >>> 'nbd': 'BlockExportOptionsNbd' > >> > >> Hm. I find it weird to have @writethrough in the base but @device in > >> the specialized class. > >> > >> I think everything that will be common to all block exports should be in > >> the base, and that probably includes a node-name. I’m aware that will > >> make things more tedious in the code, but perhaps it would be a nicer > >> interface in the end. Or is the real problem that that would create > >> problems in the storage daemon’s command line interface, because then > >> the specialized (legacy) NBD interface would no longer be compatible > >> with the new generalized block export interface? > > > > Indeed. I think patch 15 has what you're looking for. > > Great. :) > > Discussions where both participants have the same opinion from the > start are the best ones. Makes things a lot easier. Maybe I should try to move patch 15 earlier. The series is mostly just in the order that I wrote things, but there were also a few nasty dependencies in the part the generalises things from NBD to BlockExport. So I'm not sure if this is a patch that can be moved. > >> Anyway, @writable might be a similar story. A @read-only may make sense > >> in general, I think. > > > > Pulling @writable up is easier than a @read-only, but that's a naming > > detail. > > Sure. > > > In general I agree, but this part isn't addressed in this series yet. > > Part of the reason why this is an RFC was to find out if I should > > include things like this or if we'll do it when we add another export > > type or common functionality that needs the same option. > > Sure, sure. So should I or not? :-) > Meta: I personally don’t like RFC patches very much. RFC to me means > everything is fair game, and reviewers should be free to let their > thoughts wander and come up with perhaps wild ideas, just trying to > gauge what everyone thinks. > > When I’m the submitter, I tend to get defensive then, because I’ve > invested time in writing the code already, so I tend to be biased > against fundamental changes. (Horrible personal trait. I’m working > on it.) This makes sense. Nobody likes having to rewrite their RFC series. But there is one thing I dread even more: Polishing the RFC series for another week until I can send it out as non-RFC and _then_ having to rewrite it. > As a reviewer, the code and thus some fleshed out design is there > already, so it’s difficult to break free from that and find completely > different solutions to the original problem. > (I kind of ventured in that direction for this patch, and it seems like > you immediately noticed that my response was different from usual and > pointed out the RFC status, perhaps to make me feel more comfortable in > questioning the fundamental design more. Which I noticed, hence this > wall of text.) Basically just telling you that I was already interested in your input for this point specifically when I sent the series. > Perhaps I’m wrong. Perhaps it’s just myself (the points I’ve just > listed are definitely my own personal weaknesses), but I can’t help but > project and assume that others may feel similar, at least in part. > So I feel like RFCs that consist of patches tend to at least lock me in > to the solution that’s present. I find them difficult to handle, both > as a submitter and as a reviewer. > > All in all, that means on either side I tend to handle patch RFCs as > “Standard series, just tests missing”. Not sure if that’s ideal. Or > maybe that’s exactly what patch RFCs are? > > (Of course, it can and should be argued that even for standard series, I > shouldn’t be afraid of questioning the fundamental design still. But > that’s hard...) I usually send RFC patches when I know that I wouldn't consider them mergable yet, but I don't want to invest the time to polish them before I know that other people agree with the approach and the time won't be wasted. > But, well. The alternative is writing pure design RFCs, and then you > tend to get weeks of slow discussion, drawing everything out. Which > isn’t ideal either. Or is that just a baseless prejudice I have? In many cases (and I think this is one of them in large parts), I only really learn what the series will look like when I write it. I could have sent a design RFC for the QAPI part, but I didn't expect this to be contentious because it's just the normal add/del/query thing that exists for pretty much everything else, too. > >> Basically, I think that the export code should be separate from the code > >> setting up the BlockBackend that should be exported, so all options > >> regarding that BB should be common; and those options are @node-name, > >> @writethrough, and @read-only. (And perhaps other things like > >> @resizable, too, even though that isn’t something to consider for NBD.) > > > > Do you mean that the BlockBackend should already be created by the > > generic block export layer? > > It would certainly be nice, if it were feasible, don’t you think? > > We don’t have to bend backwards for it, but maybe it would force us to > bring the natural separation of block device and export parameters to > the interface. I can try. I seem to remember that you had a reason not to do this the last time we discussed generalised exports, but I'm not sure what it was. The obvious one could be that the block export layer doesn't know which permissions are needed. But it can always start with minimal permissions and let the driver do a blk_set_perm() if it needs more. Kevin
On 17.08.20 16:32, Kevin Wolf wrote: > Am 17.08.2020 um 15:51 hat Max Reitz geschrieben: >> On 17.08.20 15:13, Kevin Wolf wrote: >>> Am 17.08.2020 um 14:56 hat Max Reitz geschrieben: >>>> On 13.08.20 18:29, Kevin Wolf wrote: >>>>> qemu-nbd allows use of writethrough cache modes, which mean that write >>>>> requests made through NBD will cause a flush before they complete. >>>>> Expose the same functionality in block-export-add. >>>>> >>>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>>>> --- >>>>> qapi/block-export.json | 7 ++++++- >>>>> blockdev-nbd.c | 2 +- >>>>> 2 files changed, 7 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/qapi/block-export.json b/qapi/block-export.json >>>>> index 1fdc55c53a..4ce163411f 100644 >>>>> --- a/qapi/block-export.json >>>>> +++ b/qapi/block-export.json >>>>> @@ -167,10 +167,15 @@ >>>>> # Describes a block export, i.e. how single node should be exported on an >>>>> # external interface. >>>>> # >>>>> +# @writethrough: If true, caches are flushed after every write request to the >>>>> +# export before completion is signalled. (since: 5.2; >>>>> +# default: false) >>>>> +# >>>>> # Since: 4.2 >>>>> ## >>>>> { 'union': 'BlockExportOptions', >>>>> - 'base': { 'type': 'BlockExportType' }, >>>>> + 'base': { 'type': 'BlockExportType', >>>>> + '*writethrough': 'bool' }, >>>>> 'discriminator': 'type', >>>>> 'data': { >>>>> 'nbd': 'BlockExportOptionsNbd' >>>> >>>> Hm. I find it weird to have @writethrough in the base but @device in >>>> the specialized class. >>>> >>>> I think everything that will be common to all block exports should be in >>>> the base, and that probably includes a node-name. I’m aware that will >>>> make things more tedious in the code, but perhaps it would be a nicer >>>> interface in the end. Or is the real problem that that would create >>>> problems in the storage daemon’s command line interface, because then >>>> the specialized (legacy) NBD interface would no longer be compatible >>>> with the new generalized block export interface? >>> >>> Indeed. I think patch 15 has what you're looking for. >> >> Great. :) >> >> Discussions where both participants have the same opinion from the >> start are the best ones. > > Makes things a lot easier. > > Maybe I should try to move patch 15 earlier. The series is mostly just > in the order that I wrote things, but there were also a few nasty > dependencies in the part the generalises things from NBD to BlockExport. > So I'm not sure if this is a patch that can be moved. > >>>> Anyway, @writable might be a similar story. A @read-only may make sense >>>> in general, I think. >>> >>> Pulling @writable up is easier than a @read-only, but that's a naming >>> detail. >> >> Sure. >> >>> In general I agree, but this part isn't addressed in this series yet. >>> Part of the reason why this is an RFC was to find out if I should >>> include things like this or if we'll do it when we add another export >>> type or common functionality that needs the same option. >> >> Sure, sure. > > So should I or not? :-) Can we delay it until after this series? I.e., as long as it retains the name “writable”, would pulling it up into BlockExportOptions a compatible change? If so, then I suppose we could do it afterwards. But I think it does make the most sense to “just” do it as part of this series. >> Meta: I personally don’t like RFC patches very much. RFC to me means >> everything is fair game, and reviewers should be free to let their >> thoughts wander and come up with perhaps wild ideas, just trying to >> gauge what everyone thinks. >> >> When I’m the submitter, I tend to get defensive then, because I’ve >> invested time in writing the code already, so I tend to be biased >> against fundamental changes. (Horrible personal trait. I’m working >> on it.) > > This makes sense. Nobody likes having to rewrite their RFC series. > > But there is one thing I dread even more: Polishing the RFC series for > another week until I can send it out as non-RFC and _then_ having to > rewrite it. Yes. Especially bad with tests. >> As a reviewer, the code and thus some fleshed out design is there >> already, so it’s difficult to break free from that and find completely >> different solutions to the original problem. >> (I kind of ventured in that direction for this patch, and it seems like >> you immediately noticed that my response was different from usual and >> pointed out the RFC status, perhaps to make me feel more comfortable in >> questioning the fundamental design more. Which I noticed, hence this >> wall of text.) > > Basically just telling you that I was already interested in your input > for this point specifically when I sent the series. OK :) >> Perhaps I’m wrong. Perhaps it’s just myself (the points I’ve just >> listed are definitely my own personal weaknesses), but I can’t help but >> project and assume that others may feel similar, at least in part. >> So I feel like RFCs that consist of patches tend to at least lock me in >> to the solution that’s present. I find them difficult to handle, both >> as a submitter and as a reviewer. >> >> All in all, that means on either side I tend to handle patch RFCs as >> “Standard series, just tests missing”. Not sure if that’s ideal. Or >> maybe that’s exactly what patch RFCs are? >> >> (Of course, it can and should be argued that even for standard series, I >> shouldn’t be afraid of questioning the fundamental design still. But >> that’s hard...) > > I usually send RFC patches when I know that I wouldn't consider them > mergable yet, but I don't want to invest the time to polish them before > I know that other people agree with the approach and the time won't be > wasted. > >> But, well. The alternative is writing pure design RFCs, and then you >> tend to get weeks of slow discussion, drawing everything out. Which >> isn’t ideal either. Or is that just a baseless prejudice I have? > > In many cases (and I think this is one of them in large parts), I only > really learn what the series will look like when I write it. That’s true. With a pure design RFC, it’s often difficult to know even the scope of the design until you’ve begun to write code. So there’s a danger of just writing a bunch of uncontroversial basic design stuff because one has no idea of what may actually become problematic and questionable. :/ > I could have sent a design RFC for the QAPI part, but I didn't expect > this to be contentious because it's just the normal add/del/query thing > that exists for pretty much everything else, too. Yeah, the functions themselves are clear. Hm. Perhaps software engineering just is actually difficult, and there’s no way around it. >>>> Basically, I think that the export code should be separate from the code >>>> setting up the BlockBackend that should be exported, so all options >>>> regarding that BB should be common; and those options are @node-name, >>>> @writethrough, and @read-only. (And perhaps other things like >>>> @resizable, too, even though that isn’t something to consider for NBD.) >>> >>> Do you mean that the BlockBackend should already be created by the >>> generic block export layer? >> >> It would certainly be nice, if it were feasible, don’t you think? >> >> We don’t have to bend backwards for it, but maybe it would force us to >> bring the natural separation of block device and export parameters to >> the interface. > > I can try. I seem to remember that you had a reason not to do this the > last time we discussed generalised exports, but I'm not sure what it > was. > > The obvious one could be that the block export layer doesn't know which > permissions are needed. But it can always start with minimal permissions > and let the driver do a blk_set_perm() if it needs more. Trying sounds good. Since there shouldn’t be consequences for the QMP interface, we™ can always try again later (i.e., when adding more export types). Max
On 8/17/20 7:56 AM, Max Reitz wrote: > On 13.08.20 18:29, Kevin Wolf wrote: >> qemu-nbd allows use of writethrough cache modes, which mean that write >> requests made through NBD will cause a flush before they complete. >> Expose the same functionality in block-export-add. >> >> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >> --- >> +++ b/qapi/block-export.json >> @@ -167,10 +167,15 @@ >> # Describes a block export, i.e. how single node should be exported on an >> # external interface. >> # >> +# @writethrough: If true, caches are flushed after every write request to the >> +# export before completion is signalled. (since: 5.2; >> +# default: false) >> +# >> # Since: 4.2 >> ## >> { 'union': 'BlockExportOptions', >> - 'base': { 'type': 'BlockExportType' }, >> + 'base': { 'type': 'BlockExportType', >> + '*writethrough': 'bool' }, >> 'discriminator': 'type', >> 'data': { >> 'nbd': 'BlockExportOptionsNbd' > > Hm. I find it weird to have @writethrough in the base but @device in > the specialized class. > > I think everything that will be common to all block exports should be in > the base, and that probably includes a node-name. I’m aware that will > make things more tedious in the code, but perhaps it would be a nicer > interface in the end. Or is the real problem that that would create > problems in the storage daemon’s command line interface, because then > the specialized (legacy) NBD interface would no longer be compatible > with the new generalized block export interface? > > Anyway, @writable might be a similar story. A @read-only may make sense > in general, I think. And maybe even auto-read-only. As for @writable vs. @read-only, that's a choice of spelling, but we don't want both; there's also the question of which default is saner (having to opt-in to allowing writes is more verbose than defaulting to allowing writes when possible, but arguably saner as it is less risk of unintended writes when you forgot to specify the option; @auto-read-only can help in choosing nicer defaults). > > Basically, I think that the export code should be separate from the code > setting up the BlockBackend that should be exported, so all options > regarding that BB should be common; and those options are @node-name, > @writethrough, and @read-only. (And perhaps other things like > @resizable, too, even though that isn’t something to consider for NBD.) NBD isn't resizable yet, but extending the protocol to let it become so is on my TODO list.
On 8/13/20 11:29 AM, Kevin Wolf wrote: > qemu-nbd allows use of writethrough cache modes, which mean that write > requests made through NBD will cause a flush before they complete. > Expose the same functionality in block-export-add. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/block-export.json | 7 ++++++- > blockdev-nbd.c | 2 +- > 2 files changed, 7 insertions(+), 2 deletions(-) > > +++ b/blockdev-nbd.c > @@ -218,7 +218,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp) > > exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap, > !arg->writable, !arg->writable, > - NULL, false, errp); > + NULL, exp_args->writethrough, errp); It would someday be nice to get rid of exp_args->has_writethrough in QAPI generated code, but that's independent of this series. Meanwhile, you are safe in relying on writethrough being false when has_writethrough is false. This change makes sense to me interface-wise. QAPI-wise, should we try harder to make writethrough settable only for writable exports (and an error for read-only exports)? I'm not sure what QAPI contortions would have to look like to make that enforced by the QMP parser; but it's also not the end of the world if we just always permit the optional field and then apply a post-parse semantic check.
diff --git a/qapi/block-export.json b/qapi/block-export.json index 1fdc55c53a..4ce163411f 100644 --- a/qapi/block-export.json +++ b/qapi/block-export.json @@ -167,10 +167,15 @@ # Describes a block export, i.e. how single node should be exported on an # external interface. # +# @writethrough: If true, caches are flushed after every write request to the +# export before completion is signalled. (since: 5.2; +# default: false) +# # Since: 4.2 ## { 'union': 'BlockExportOptions', - 'base': { 'type': 'BlockExportType' }, + 'base': { 'type': 'BlockExportType', + '*writethrough': 'bool' }, 'discriminator': 'type', 'data': { 'nbd': 'BlockExportOptionsNbd' diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 28159a92b2..17417c1b6b 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -218,7 +218,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp) exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap, !arg->writable, !arg->writable, - NULL, false, errp); + NULL, exp_args->writethrough, errp); if (!exp) { goto out; }
qemu-nbd allows use of writethrough cache modes, which mean that write requests made through NBD will cause a flush before they complete. Expose the same functionality in block-export-add. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- qapi/block-export.json | 7 ++++++- blockdev-nbd.c | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-)