Xen devel - Aug 2012 - [PATCH v3] 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 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..5ffc334 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 "No image or block device found in $xpath/params"
+else
+	error "Invalid file type for block device." \
+	      "Only block and regular image files accepted."
+fi
 
 case $xstatus in
 6)
-- 
1.7.7.5 (Apple Git-26)

On Tue, 2012-08-14 at 16:06 +0100, 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 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..5ffc334 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 "No image or block device found in $xpath/params"
> +else
> +	error "Invalid file type for block device." \
> +	      "Only block and regular image files accepted."
Perhaps include $xparams in here somewhere? Perhaps $xpath too?
> +fi
>  
>  case $xstatus in
>  6)

Ian Campbell wrote:
> On Tue, 2012-08-14 at 16:06 +0100, 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 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..5ffc334 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 "No image or block device found in $xpath/params"
>> +else
>> +	error "Invalid file type for block device." \
>> +	      "Only block and regular image files accepted."
> 
> Perhaps include $xparams in here somewhere? Perhaps $xpath too?
Thanks for the review.

I think including $xparams should be enough (since it is not null).