Re: [Xen-devel] [PATCH] xen-blk(front|back): Handle large physical sector disks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.05.13 at 12:04, James Harper <james.harper@xxxxxxxxxxxxxxxx> wrote:
>>  
>> >>> On 15.05.13 at 11:26, Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>> wrote:
>> > On 14.05.2013 10:04, Jan Beulich wrote:
>> >>>>> On 13.05.13 at 19:47, Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>> wrote:
>> >>> --- a/drivers/block/xen-blkback/xenbus.c
>> >>> +++ b/drivers/block/xen-blkback/xenbus.c
>> >>> @@ -704,6 +704,13 @@ again:
>> >>>                                   dev->nodename);
>> >>>                  goto abort;
>> >>>          }
>> >>> +        err = xenbus_printf(xbt, dev->nodename, "physical-sector-size",
>> "%u",
>> >>> +                            
>> >>> bdev_physical_block_size(be->blkif->vbd.bdev));
>> >>> +        if (err) {
>> >>> +                xenbus_dev_fatal(dev, err, "writing 
>> >>> %s/physical-sector-size",
>> >>> +                                 dev->nodename);
>> >>> +                goto abort;
>> >>
>> >> Failure here should not be fatal (as with any other protocol
>> >> extensions).
>> >
>> > So I suppose that should be xenbus_dev_error and no abort here. Just
>> > wondering
>> > (and sorry for being thick headed here) why would a failure here be
>> > different in
>> > severity for an extension or not. Is that not just adding an element to the
>> > xenstore object and failure would not be related to this being an
>> extension?
>> 
>> A driver should only bail upon encountering a problem that it can't
>> recover from. Failure to write a xenstore node that neither the
>> backend nor the frontend really require for their work is certainly
>> not among those. Yes, it's only a xenstore write, but it can fail at
>> least theoretically (or else there wouldn't be a need for error
>> handling here in the first place), and you shouldn't handle such
>> failure in undue ways (i.e. failure to write required nodes is fatal,
>> but failure to write nodes related to extensions isn't).
>> 
> 
> What is the recovery though? If the physical block size is unusual (eg not 
> 512) and the write has failed, what is the outcome? I suspect that it's going 
> to not be what the user expected - partitions could be incorrectly aligned if 
> doing an install, etc. If it were my system then in the (vanishingly rare?) 
> case that this write failed, I'd prefer a hard failure.

That's a policy decision, but the driver should not enforce policy.
And remember, till now this information isn't even being passed,
so the question of hard failure being preferable is pretty mute.

> If a simple write to xenstore fails then isn't the world coming to an end 
> anyway?

Possibly, albeit xenstore could fail this just because there's no
xenstore space left for one of the involved entities.

And again, my main motivation to request the change is to not
set bad precedents. The error here conceptually should not be
fatal, with no regard to the actual functionality it represents.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel