Roger Pau Monne
2012-Aug-14 18:09 UTC
[PATCH v4] hotplug/NetBSD: check type of file to attach from params
xend used to set the xenbus backend entry "type" to either "phy" or "file", but now libxl sets it to "phy" for both file and block device. We have to manually check for the type of the "param" field in order to detect if we are trying to attach a file or a block device. Cc: Ian Jackson <ian.jackson@eu.citrix.com> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com> --- Changes since v3: * Add $xparams (that contains the path to the disk file) to the error message. Changes since v2: * Better error messages. * Check if params is empty. * Replace xenstore_write with xenstore-write in error function. * Add quotation marks to xparams when testing. Changes since v1: * Check that file is either a block special file or a regular file and report error otherwise. --- tools/hotplug/NetBSD/block | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tools/hotplug/NetBSD/block b/tools/hotplug/NetBSD/block index cf5ff3a..f849fe4 100644 --- a/tools/hotplug/NetBSD/block +++ b/tools/hotplug/NetBSD/block @@ -12,15 +12,24 @@ export PATH error() { echo "$@" >&2 - xenstore_write $xpath/hotplug-status error + xenstore-write $xpath/hotplug-status error exit 1 } xpath=$1 xstatus=$2 -xtype=$(xenstore-read "$xpath/type") xparams=$(xenstore-read "$xpath/params") +if [ -b "$xparams" ]; then + xtype="phy" +elif [ -f "$xparams" ]; then + xtype="file" +elif [ -z "$xparams" ]; then + error "$xpath/params is empty, unable to attach block device." +else + error "$xparams is not a valid file type to use as block device." \ + "Only block and regular image files accepted." +fi case $xstatus in 6) -- 1.7.7.5 (Apple Git-26)
Christoph Egger
2012-Aug-15 08:42 UTC
Re: [PATCH v4] hotplug/NetBSD: check type of file to attach from params
Fine with me. Christoph On 08/14/12 20:09, Roger Pau Monne wrote:> xend used to set the xenbus backend entry "type" to either "phy" or > "file", but now libxl sets it to "phy" for both file and block device. > We have to manually check for the type of the "param" field in order > to detect if we are trying to attach a file or a block device. > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> > Signed-off-by: Roger Pau Monne <roger.pau@citrix.com> > --- > Changes since v3: > > * Add $xparams (that contains the path to the disk file) to the error > message. > > Changes since v2: > > * Better error messages. > > * Check if params is empty. > > * Replace xenstore_write with xenstore-write in error function. > > * Add quotation marks to xparams when testing. > > Changes since v1: > > * Check that file is either a block special file or a regular file > and report error otherwise. > --- > tools/hotplug/NetBSD/block | 13 +++++++++++-- > 1 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/tools/hotplug/NetBSD/block b/tools/hotplug/NetBSD/block > index cf5ff3a..f849fe4 100644 > --- a/tools/hotplug/NetBSD/block > +++ b/tools/hotplug/NetBSD/block > @@ -12,15 +12,24 @@ export PATH > > error() { > echo "$@" >&2 > - xenstore_write $xpath/hotplug-status error > + xenstore-write $xpath/hotplug-status error > exit 1 > } > > > xpath=$1 > xstatus=$2 > -xtype=$(xenstore-read "$xpath/type") > xparams=$(xenstore-read "$xpath/params") > +if [ -b "$xparams" ]; then > + xtype="phy" > +elif [ -f "$xparams" ]; then > + xtype="file" > +elif [ -z "$xparams" ]; then > + error "$xpath/params is empty, unable to attach block device." > +else > + error "$xparams is not a valid file type to use as block device." \ > + "Only block and regular image files accepted." > +fi > > case $xstatus in > 6)-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
Ian Campbell
2012-Aug-17 08:13 UTC
Re: [PATCH v4] hotplug/NetBSD: check type of file to attach from params
On Tue, 2012-08-14 at 19:09 +0100, Roger Pau Monne wrote:> Changes since v2:[...]> * Replace xenstore_write with xenstore-write in error function.[...]> error() { > echo "$@" >&2 > - xenstore_write $xpath/hotplug-status error > + xenstore-write $xpath/hotplug-status error > exit 1 > }Why this seemingly unrelated change? I don''t see anything in the comments on v2 explicitly about it. If it is somehow necessary due to this patch then I think that deserves mention in the changelog proper. Is it because xenstore_write is actually specific to the Linux hotplug scripts? (i.e. this function was just plain broken before). While looking into this I noticed that the Linux equivalent to error() is: fatal() { _xenstore_write "$XENBUS_PATH/hotplug-error" "$*" \ "$XENBUS_PATH/hotplug-status" error log err "$@" exit 1 } The write of the log message to hotplus-error seems like something worth replicating (in a separate patch). I also wonder how much of this sort of infrastructure could actually be shared, but that''s for another time. Ian.
Christoph Egger
2012-Aug-17 08:25 UTC
Re: [PATCH v4] hotplug/NetBSD: check type of file to attach from params
On 08/17/12 10:13, Ian Campbell wrote:> On Tue, 2012-08-14 at 19:09 +0100, Roger Pau Monne wrote: >> Changes since v2: > [...] >> * Replace xenstore_write with xenstore-write in error function. > [...] >> error() { >> echo "$@" >&2 >> - xenstore_write $xpath/hotplug-status error >> + xenstore-write $xpath/hotplug-status error >> exit 1 >> } > > Why this seemingly unrelated change? I don''t see anything in the > comments on v2 explicitly about it.xenstore-write exists on NetBSD. xenstore_write does not exist. Christoph> > If it is somehow necessary due to this patch then I think that deserves > mention in the changelog proper. > > Is it because xenstore_write is actually specific to the Linux hotplug > scripts? (i.e. this function was just plain broken before). > > While looking into this I noticed that the Linux equivalent to error() > is: > fatal() { > _xenstore_write "$XENBUS_PATH/hotplug-error" "$*" \ > "$XENBUS_PATH/hotplug-status" error > log err "$@" > exit 1 > } > > The write of the log message to hotplus-error seems like something worth > replicating (in a separate patch). > > I also wonder how much of this sort of infrastructure could actually be > shared, but that''s for another time. > > Ian. > >-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632