[Kimchi-devel] [PATCH] Issue #999: Attach storage to guest on s390x without libvirt
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Sep 19 16:49:14 UTC 2016
On 09/19/2016 12:33 PM, Harshal Patil wrote:
> Harshal Patil
> E-mail: harshal.patil at in.ibm.com <mailto:harshal.patil at in.ibm.com>
>
> ----- Original message -----
> From: Aline Manera <alinefm at linux.vnet.ibm.com>
> Sent by: kimchi-devel-bounces at ovirt.org
> To: harshalp at linux.vnet.ibm.com, Kimchi Devel <kimchi-devel at ovirt.org>
> Cc:
> Subject: Re: [Kimchi-devel] [PATCH] Issue #999: Attach storage to
> guest on s390x without libvirt
> Date: Sat, Sep 17, 2016 1:03 AM
>
> On 09/12/2016 06:55 AM, harshalp at linux.vnet.ibm.com wrote:
> > From: Harshal Patil <harshalp at linux.vnet.ibm.com>
> >
> > This patch enables direct storage attachment to guests running
> > on s390x arch without going through libvirt
> >
> > Signed-off-by: Harshal Patil <harshalp at linux.vnet.ibm.com>
> > ---
> > docs/API.md | 3 +++
> > i18n.py | 3 +++
> > model/vmstorages.py | 25 +++++++++++++++++++++++--
> > utils.py | 2 +-
> > 4 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/docs/API.md b/docs/API.md
> > index 6678ac5..c9d5d2a 100644
> > --- a/docs/API.md
> > +++ b/docs/API.md
> > @@ -239,6 +239,9 @@ Represents a snapshot of the Virtual
> Machine's primary monitor.
> > * path: Path of cdrom iso.
> > * pool: Storage pool which disk image file locate in.
> > * vol: Storage volume name of disk image.
> > + * dir_path: s390x specific attribute to attach direct
> storage without libvirt
> > + * format: s390x specific attribute specify the format of
> direct storage
> > + * size: s390x specific attribute to specify the size of
> direct storage
> >
> > ### Sub-resource: storage
> > **URI:** /plugins/kimchi/vms/*:name*/storages/*:dev*
> > diff --git a/i18n.py b/i18n.py
> > index fc7dbc7..ceb4286 100644
> > --- a/i18n.py
> > +++ b/i18n.py
> > @@ -332,6 +332,9 @@ messages = {
> > "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": _("Volume chosen with format %(format)s
> does not fit in the storage type %(type)s"),
> > + "KCHVMSTOR0019E": _("On s390x arch one of pool, path of
> dir_path must be specified"),
> > + "KCHVMSTOR0020E": _("On s390x arch 'format' must be
> specified while attaching disk to virtual machine"),
> > + "KCHVMSTOR0021E": _("Virtual disk already exists on the
> system: %(disk_path)s"),
> >
> > "KCHSNAP0001E": _("Virtual machine '%(vm)s' must be
> stopped before creating a snapshot of it."),
> > "KCHSNAP0002E": _("Unable to create snapshot '%(name)s' on
> virtual machine '%(vm)s'. Details: %(err)s"),
> > diff --git a/model/vmstorages.py b/model/vmstorages.py
> > index 2e9f783..156a16f 100644
> > --- a/model/vmstorages.py
> > +++ b/model/vmstorages.py
> > @@ -17,6 +17,7 @@
> > # License along with this library; if not, write to the Free
> Software
> > # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> MA 02110-1301 USA
> >
> > +import os
> > import string
> > from lxml import etree
> >
> > @@ -30,6 +31,7 @@ from wok.plugins.kimchi.model.storagevolumes
> import StorageVolumeModel
> > from wok.plugins.kimchi.model.utils import get_vm_config_flag
> > from wok.plugins.kimchi.model.vms import DOM_STATE_MAP, VMModel
> > from wok.plugins.kimchi.osinfo import lookup
> > +from wok.plugins.kimchi.utils import create_disk_image, is_s390x
> > from wok.plugins.kimchi.xmlutils.disk import get_device_node,
> get_disk_xml
> > from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info,
> get_vm_disks
> >
> > @@ -82,11 +84,30 @@ class VMStoragesModel(object):
> > # Path will never be blank due to API.json verification.
> > # There is no need to cover this case here.
> > if not ('vol' in params) ^ ('path' in params):
> > - raise InvalidParameter("KCHVMSTOR0017E")
> > + if not is_s390x():
> > + raise InvalidParameter("KCHVMSTOR0017E")
> > +
>
> > + if 'dir_path' not in params:
> > + raise InvalidParameter("KCHVMSTOR0019E")
>
> >The dir_path is exclusively for s390x. You need to add this
> verification
> >too.
>
> Not required. Check the condition again (The line which says 'if not
> is_s390x()'). Only if the platform is s390x it will enter the code
> path where 'dir_path' is checked so there is no need to check for
> s390x arch again. I have tested this flow on x86 as well as on s390x,
> and it works fine.
Sorry but how the first condition is related to the second one?!
If I am x86 and I have specified a 'vol' or a 'path' and not a
'dir_path' as it is exclusively for s390x, it will raise the dir_path
exception (the second condition).
So or you add a is_s390x() condition to the second statement or you
place the whole first block in a if conduction:
if not is390x():
if not (vol in params) ^ (path in params):
(...)
else:
if dir_path not in params:
raise ...
>
>
> > dom = VMModel.get_vm(vm_name, self.conn)
> > params['bus'] = _get_device_bus(params['type'], dom)
> > - params['format'] = 'raw'
> > +
> > + if is_s390x() and params['type'] == 'disk':
> > + if 'format' not in params:
> > + raise InvalidParameter("KCHVMSTOR0020E")
> > + if 'dir_path' in params:
> > + size = params['size']
> > + name = params['name']
> > + dir_path = params.get('dir_path')
> > + params['path'] = dir_path + "/" + name
> > + if os.path.exists(params['path']):
> > + raise InvalidParameter("KCHVMSTOR0021E",
> > + {'disk_path': params['path']})
> > + create_disk_image(format_type=params['format'],
> > + path=params['path'], capacity=size)
> > + else:
> > + params['format'] = 'raw'
> >
> > dev_list = [dev for dev, bus in
> get_vm_disks(dom).iteritems()
> > if bus == params['bus']]
> > diff --git a/utils.py b/utils.py
> > index 0fca191..7fdec02 100644
> > --- a/utils.py
> > +++ b/utils.py
> > @@ -305,4 +305,4 @@ def create_disk_image(format_type, path,
> capacity):
> > if rc != 0:
> > raise OperationFailed("KCHTMPL0035E", {'err': err})
> >
> > - return
> > \ No newline at end of file
> > + return
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20160919/921964cf/attachment.html>
More information about the Kimchi-devel
mailing list