Kimchi does not allow scsi devices to be attached in a VM (devices with names sdb,sdc ...). A simple change in API.json regex of vmstorages_create fixes it. At the same time, Power systems does not support IDE devices and adding devices with 'hd[b-z]' name have unpredictable results. This patch also adds a backend verification to avoid this scenario. Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; --- src/kimchi/API.json | 2 +- src/kimchi/i18n.py | 1 + src/kimchi/model/vmstorages.py | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 1319531..4a3cd03 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -489,7 +489,7 @@ "dev": { "description": "The storage device name", "type": "string", - "pattern": "^h|vd[b-z]$", + "pattern": "^h|s|vd[b-z]$", "error": "KCHVMSTOR0001E" }, "type": { diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 9e66c68..0839007 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -259,6 +259,7 @@ messages = { "KCHVMSTOR0015E": _("Cannot retrieve disk path information for given pool/volume: %(error)s"), "KCHVMSTOR0016E": _("Volume already in use by other virtual machine."), "KCHVMSTOR0017E": _("Only one of path or pool/volume can be specified to add a new virtual machine disk"), + "KCHVMSTOR0018E": _("Device name %(dev_name)s is not allowed because Power systems does not support IDE devices"), "KCHREPOS0001E": _("YUM Repository ID must be one word only string."), "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or file:// URL."), diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 40856d3..c302709 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -18,6 +18,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import os +import platform +import re import socket import stat import string @@ -188,6 +190,11 @@ class VMStoragesModel(object): string.ascii_lowercase.index(last_dev[2]) + 1 params['dev'] =\ bus_prefix + string.ascii_lowercase[next_dev_letter_pos] + # Power systems does not support IDE devices + elif platform.machine().startswith('ppc') and \ + re.match('^hd[b-z]', params.get('dev')): + raise OperationFailed('KCHVMSTOR0018E', + {'dev_name': params['dev']}) devices = self.get_list(vm_name) if params['dev'] in devices:

On 09/16/2014 12:02 PM, Daniel Henrique Barboza wrote: > Kimchi does not allow scsi devices to be attached in a VM (devices > with names sdb,sdc ...). A simple change in API.json regex of > vmstorages_create fixes it. > > At the same time, Power systems does not support IDE devices and > adding devices with 'hd[b-z]' name have unpredictable results. > This patch also adds a backend verification to avoid this scenario. > > Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; > --- > src/kimchi/API.json | 2 +- > src/kimchi/i18n.py | 1 + > src/kimchi/model/vmstorages.py | 7 +++++++ > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/kimchi/API.json b/src/kimchi/API.json > index 1319531..4a3cd03 100644 > --- a/src/kimchi/API.json > +++ b/src/kimchi/API.json > @@ -489,7 +489,7 @@ > "dev": { > "description": "The storage device name", > "type": "string", > - "pattern": "^h|vd[b-z]$", > + "pattern": "^h|s|vd[b-z]$", > "error": "KCHVMSTOR0001E" > }, > "type": { > diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py > index 9e66c68..0839007 100644 > --- a/src/kimchi/i18n.py > +++ b/src/kimchi/i18n.py > @@ -259,6 +259,7 @@ messages = { > "KCHVMSTOR0015E": _("Cannot retrieve disk path information for > given pool/volume: %(error)s"), > "KCHVMSTOR0016E": _("Volume already in use by other virtual > machine."), > "KCHVMSTOR0017E": _("Only one of path or pool/volume can be > specified to add a new virtual machine disk"), > + "KCHVMSTOR0018E": _("Device name %(dev_name)s is not allowed > because Power systems does not support IDE devices"), > > "KCHREPOS0001E": _("YUM Repository ID must be one word only > string."), > "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// > or file:// URL."), > diff --git a/src/kimchi/model/vmstorages.py > b/src/kimchi/model/vmstorages.py > index 40856d3..c302709 100644 > --- a/src/kimchi/model/vmstorages.py > +++ b/src/kimchi/model/vmstorages.py > @@ -18,6 +18,8 @@ > # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > > import os > +import platform > +import re > import socket > import stat > import string > @@ -188,6 +190,11 @@ class VMStoragesModel(object): > string.ascii_lowercase.index(last_dev[2]) + 1 > params['dev'] =\ > bus_prefix + > string.ascii_lowercase[next_dev_letter_pos] > + # Power systems does not support IDE devices > + elif platform.machine().startswith('ppc') and \ > + re.match('^hd[b-z]', params.get('dev')): > + raise OperationFailed('KCHVMSTOR0018E', > + {'dev_name': params['dev']}) From previous Royce's comments, I understood she asked you to add a verification based on bus type, independent of platforms. if bus == ide: # name must start with hdX if bus == scsi: # name must start with sdX if bus == virtio: # name must start with vdX > devices = self.get_list(vm_name) > if params['dev'] in devices: _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 09/16/2014 09:20 PM, Aline Manera wrote: > > And a bigger concern, why are we asking for the device name? It > should not be prompted to user. > If you are option, I suggest removing this option. If you are open... =)

> > On 09/16/2014 09:16 PM, Aline Manera wrote: >> >> On 09/16/2014 12:02 PM, Daniel Henrique Barboza wrote: >>> Kimchi does not allow scsi devices to be attached in a VM (devices >>> with names sdb,sdc ...). A simple change in API.json regex of >>> vmstorages_create fixes it. >>> >>> At the same time, Power systems does not support IDE devices and >>> adding devices with 'hd[b-z]' name have unpredictable results. >>> This patch also adds a backend verification to avoid this scenario. >>> >>> Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; >>> --- >>> src/kimchi/API.json | 2 +- >>> src/kimchi/i18n.py | 1 + >>> src/kimchi/model/vmstorages.py | 7 +++++++ >>> 3 files changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/kimchi/API.json b/src/kimchi/API.json >>> index 1319531..4a3cd03 100644 >>> --- a/src/kimchi/API.json >>> +++ b/src/kimchi/API.json >>> @@ -489,7 +489,7 @@ >>> "dev": { >>> "description": "The storage device name", >>> "type": "string", >>> - "pattern": "^h|vd[b-z]$", >>> + "pattern": "^h|s|vd[b-z]$", >>> "error": "KCHVMSTOR0001E" >>> }, >>> "type": { >>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py >>> index 9e66c68..0839007 100644 >>> --- a/src/kimchi/i18n.py >>> +++ b/src/kimchi/i18n.py >>> @@ -259,6 +259,7 @@ messages = { >>> "KCHVMSTOR0015E": _("Cannot retrieve disk path information >>> for given pool/volume: %(error)s"), >>> "KCHVMSTOR0016E": _("Volume already in use by other virtual >>> machine."), >>> "KCHVMSTOR0017E": _("Only one of path or pool/volume can be >>> specified to add a new virtual machine disk"), >>> + "KCHVMSTOR0018E": _("Device name %(dev_name)s is not allowed >>> because Power systems does not support IDE devices"), >>> >>> "KCHREPOS0001E": _("YUM Repository ID must be one word only >>> string."), >>> "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// >>> or file:// URL."), >>> diff --git a/src/kimchi/model/vmstorages.py >>> b/src/kimchi/model/vmstorages.py >>> index 40856d3..c302709 100644 >>> --- a/src/kimchi/model/vmstorages.py >>> +++ b/src/kimchi/model/vmstorages.py >>> @@ -18,6 +18,8 @@ >>> # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >>> 02110-1301 USA >>> >>> import os >>> +import platform >>> +import re >>> import socket >>> import stat >>> import string >>> @@ -188,6 +190,11 @@ class VMStoragesModel(object): >>> string.ascii_lowercase.index(last_dev[2]) + 1 >>> params['dev'] =\ >>> bus_prefix + >>> string.ascii_lowercase[next_dev_letter_pos] >> >> >>> + # Power systems does not support IDE devices >>> + elif platform.machine().startswith('ppc') and \ >>> + re.match('^hd[b-z]', params.get('dev')): >>> + raise OperationFailed('KCHVMSTOR0018E', >>> + {'dev_name': params['dev']}) >> >> From previous Royce's comments, I understood she asked you to add a >> verification based on bus type, independent of platforms. >> >> if bus == ide: >> # name must start with hdX >> if bus == scsi: >> # name must start with sdX >> if bus == virtio: >> # name must start with vdX >> >>> devices = self.get_list(vm_name) >>> if params['dev'] in devices: >> >> _______________________________________________ >> Kimchi-devel mailing list >> Kimchi-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >

On 2014年09月17日 09:44, Daniel H Barboza wrote: > > On 09/16/2014 09:22 PM, Aline Manera wrote: >> >> On 09/16/2014 09:20 PM, Aline Manera wrote: >>> >>> And a bigger concern, why are we asking for the device name? It >>> should not be prompted to user. >>> If you are option, I suggest removing this option. >> >> If you are open... =) > > I thought about the possibilty of removing the device name entirely. > It would simplify a lot of things. > > If no one objects I can remove it from the UI Good for me. >> >>> >>> On 09/16/2014 09:16 PM, Aline Manera wrote: >>>> >>>> On 09/16/2014 12:02 PM, Daniel Henrique Barboza wrote: >>>>> Kimchi does not allow scsi devices to be attached in a VM (devices >>>>> with names sdb,sdc ...). A simple change in API.json regex of >>>>> vmstorages_create fixes it. >>>>> >>>>> At the same time, Power systems does not support IDE devices and >>>>> adding devices with 'hd[b-z]' name have unpredictable results. >>>>> This patch also adds a backend verification to avoid this scenario. >>>>> >>>>> Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; >>>>> --- >>>>> src/kimchi/API.json | 2 +- >>>>> src/kimchi/i18n.py | 1 + >>>>> src/kimchi/model/vmstorages.py | 7 +++++++ >>>>> 3 files changed, 9 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/src/kimchi/API.json b/src/kimchi/API.json >>>>> index 1319531..4a3cd03 100644 >>>>> --- a/src/kimchi/API.json >>>>> +++ b/src/kimchi/API.json >>>>> @@ -489,7 +489,7 @@ >>>>> "dev": { >>>>> "description": "The storage device name", >>>>> "type": "string", >>>>> - "pattern": "^h|vd[b-z]$", >>>>> + "pattern": "^h|s|vd[b-z]$", >>>>> "error": "KCHVMSTOR0001E" >>>>> }, >>>>> "type": { >>>>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py >>>>> index 9e66c68..0839007 100644 >>>>> --- a/src/kimchi/i18n.py >>>>> +++ b/src/kimchi/i18n.py >>>>> @@ -259,6 +259,7 @@ messages = { >>>>> "KCHVMSTOR0015E": _("Cannot retrieve disk path information for >>>>> given pool/volume: %(error)s"), >>>>> "KCHVMSTOR0016E": _("Volume already in use by other virtual >>>>> machine."), >>>>> "KCHVMSTOR0017E": _("Only one of path or pool/volume can be >>>>> specified to add a new virtual machine disk"), >>>>> + "KCHVMSTOR0018E": _("Device name %(dev_name)s is not allowed >>>>> because Power systems does not support IDE devices"), >>>>> >>>>> "KCHREPOS0001E": _("YUM Repository ID must be one word only >>>>> string."), >>>>> "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or >>>>> file:// URL."), >>>>> diff --git a/src/kimchi/model/vmstorages.py >>>>> b/src/kimchi/model/vmstorages.py >>>>> index 40856d3..c302709 100644 >>>>> --- a/src/kimchi/model/vmstorages.py >>>>> +++ b/src/kimchi/model/vmstorages.py >>>>> @@ -18,6 +18,8 @@ >>>>> # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >>>>> 02110-1301 USA >>>>> >>>>> import os >>>>> +import platform >>>>> +import re >>>>> import socket >>>>> import stat >>>>> import string >>>>> @@ -188,6 +190,11 @@ class VMStoragesModel(object): >>>>> string.ascii_lowercase.index(last_dev[2]) + 1 >>>>> params['dev'] =\ >>>>> bus_prefix + string.ascii_lowercase[next_dev_letter_pos] >>>> >>>> >>>>> + # Power systems does not support IDE devices >>>>> + elif platform.machine().startswith('ppc') and \ >>>>> + re.match('^hd[b-z]', params.get('dev')): >>>>> + raise OperationFailed('KCHVMSTOR0018E', >>>>> + {'dev_name': params['dev']}) >>>> >>>> From previous Royce's comments, I understood she asked you to add >>>> a verification based on bus type, independent of platforms. >>>> >>>> if bus == ide: >>>> # name must start with hdX >>>> if bus == scsi: >>>> # name must start with sdX >>>> if bus == virtio: >>>> # name must start with vdX >>>> >>>>> devices = self.get_list(vm_name) >>>>> if params['dev'] in devices: >>>> >>>> _______________________________________________ >>>> Kimchi-devel mailing list >>>> Kimchi-devel(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>>> >>> >>> _______________________________________________ >>> Kimchi-devel mailing list >>> Kimchi-devel(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>> >> > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >