1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] qemu: command: align disk serial check to schema

Nikolay Shirokovskiy posted 1 patch 7 years ago
Failed in applying to current master (apply log)
src/qemu/qemu_command.c                              | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml  | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
[libvirt] [PATCH v2] qemu: command: align disk serial check to schema
Posted by Nikolay Shirokovskiy 7 years ago 
Disk serial schema has extra '.+' allowed characters in comparison
with check in code. Looks like there is no reason for that as qemu
allows any character AFAIK for serial. This discrepancy is originated
in 85d15b51 where ability to add serial was added.
---

Diff from v1:

* fix xml2argv disk serial test to use all valid chars

Looks like there is no existing infrastructure to test every invalid character.

 src/qemu/qemu_command.c                              | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c76f923..c5369b0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -432,7 +432,7 @@ qemuBuildIoEventFdStr(virBufferPtr buf,
 }
 
 #define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
-  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
+  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+"
 
 static int
 qemuSafeSerialParamValue(const char *value)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
index 2cefdca..fa0fc93 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
@@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \
 -boot c \
 -usb \
 -drive 'file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-1,\
-serial=\ \ WD-WMAP9A966149' \
+serial=abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_\ .+' \
 -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
index 565462e..d54d73b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
@@ -17,7 +17,7 @@
     <disk type='block' device='disk'>
       <source dev='/dev/HostVG/QEMUGuest1'/>
       <target dev='hda' bus='ide'/>
-      <serial>  WD-WMAP9A966149</serial>
+      <serial>abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+</serial>
       <address type='drive' controller='0' bus='0' target='0' unit='1'/>
     </disk>
     <controller type='usb' index='0'/>
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: command: align disk serial check to schema
Posted by Nikolay Shirokovskiy 7 years ago 
ping

On 28.03.2017 11:10, Nikolay Shirokovskiy wrote:
> Disk serial schema has extra '.+' allowed characters in comparison
> with check in code. Looks like there is no reason for that as qemu
> allows any character AFAIK for serial. This discrepancy is originated
> in 85d15b51 where ability to add serial was added.
> ---
> 
> Diff from v1:
> 
> * fix xml2argv disk serial test to use all valid chars
> 
> Looks like there is no existing infrastructure to test every invalid character.
> 
>  src/qemu/qemu_command.c                              | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c76f923..c5369b0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -432,7 +432,7 @@ qemuBuildIoEventFdStr(virBufferPtr buf,
>  }
>  
>  #define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
> -  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
> +  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+"
>  
>  static int
>  qemuSafeSerialParamValue(const char *value)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> index 2cefdca..fa0fc93 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> @@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \
>  -boot c \
>  -usb \
>  -drive 'file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-1,\
> -serial=\ \ WD-WMAP9A966149' \
> +serial=abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_\ .+' \
>  -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
>  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> index 565462e..d54d73b 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> @@ -17,7 +17,7 @@
>      <disk type='block' device='disk'>
>        <source dev='/dev/HostVG/QEMUGuest1'/>
>        <target dev='hda' bus='ide'/>
> -      <serial>  WD-WMAP9A966149</serial>
> +      <serial>abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+</serial>
>        <address type='drive' controller='0' bus='0' target='0' unit='1'/>
>      </disk>
>      <controller type='usb' index='0'/>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: command: align disk serial check to schema
Posted by John Ferlan 6 years, 8 months ago 

On 03/28/2017 04:10 AM, Nikolay Shirokovskiy wrote:
> Disk serial schema has extra '.+' allowed characters in comparison
> with check in code. Looks like there is no reason for that as qemu
> allows any character AFAIK for serial. This discrepancy is originated
> in 85d15b51 where ability to add serial was added.
> ---
> 
> Diff from v1:
> 
> * fix xml2argv disk serial test to use all valid chars
> 
> Looks like there is no existing infrastructure to test every invalid character.
> 
>  src/qemu/qemu_command.c                              | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

Doing some (personal) mail box cleaning and found this "older" patch...
Sorry that this has sat unattended (and probably now forgotten or given
up on) for so long.

In any case, I adjusted the test to add a second disk rather than
modifying the first one which was added as a result of commit id
'5aee81a0c'.

Reviewed-by: John Ferlan <jferlan@redhat.com>

I'll push shortly.


John

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c76f923..c5369b0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -432,7 +432,7 @@ qemuBuildIoEventFdStr(virBufferPtr buf,
>  }
>  
>  #define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
> -  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
> +  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+"
>  
>  static int
>  qemuSafeSerialParamValue(const char *value)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> index 2cefdca..fa0fc93 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> @@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \
>  -boot c \
>  -usb \
>  -drive 'file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-1,\
> -serial=\ \ WD-WMAP9A966149' \
> +serial=abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_\ .+' \
>  -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
>  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> index 565462e..d54d73b 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> @@ -17,7 +17,7 @@
>      <disk type='block' device='disk'>
>        <source dev='/dev/HostVG/QEMUGuest1'/>
>        <target dev='hda' bus='ide'/>
> -      <serial>  WD-WMAP9A966149</serial>
> +      <serial>abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+</serial>
>        <address type='drive' controller='0' bus='0' target='0' unit='1'/>
>      </disk>
>      <controller type='usb' index='0'/>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: command: align disk serial check to schema
Posted by Nikolay Shirokovskiy 6 years, 7 months ago 

On 03.08.2017 02:24, John Ferlan wrote:
> 
> 
> On 03/28/2017 04:10 AM, Nikolay Shirokovskiy wrote:
>> Disk serial schema has extra '.+' allowed characters in comparison
>> with check in code. Looks like there is no reason for that as qemu
>> allows any character AFAIK for serial. This discrepancy is originated
>> in 85d15b51 where ability to add serial was added.
>> ---
>>
>> Diff from v1:
>>
>> * fix xml2argv disk serial test to use all valid chars
>>
>> Looks like there is no existing infrastructure to test every invalid character.
>>
>>  src/qemu/qemu_command.c                              | 2 +-
>>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args | 2 +-
>>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml  | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
> 
> Doing some (personal) mail box cleaning and found this "older" patch...
> Sorry that this has sat unattended (and probably now forgotten or given
> up on) for so long.
> 
> In any case, I adjusted the test to add a second disk rather than
> modifying the first one which was added as a result of commit id
> '5aee81a0c'.
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> I'll push shortly.
> 
> 

Thanx!

Nikolay

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.