[PATCH v4] hotplug/NetBSD: check type of file to attach from params

[PATCH v4] hotplug/NetBSD: check type of file to attach from params

[Roger Pau Monne]

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)

Re: [PATCH v4] hotplug/NetBSD: check type of file to attach from params

[Christoph Egger]

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

Re: [PATCH v4] hotplug/NetBSD: check type of file to attach from params

[Ian Campbell]

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.

Re: [PATCH v4] hotplug/NetBSD: check type of file to attach from params

[Christoph Egger]

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