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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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).

Jan


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