<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 09/19/2016 10:19 PM, Aline Manera
wrote:<br>
</div>
<blockquote
cite="mid:31ba4c43-9a32-ee62-8723-26050fdebc48@linux.vnet.ibm.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<br>
<br>
<div class="moz-cite-prefix">On 09/19/2016 12:33 PM, Harshal Patil
wrote:<br>
</div>
<blockquote
cite="mid:OF5727B727.96B7C675-ON00258033.0054FCC7-00258033.005575D0@notes.na.collabserv.com"
type="cite">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial;font-size:10.5pt">
<div dir="ltr"> </div>
<div dir="ltr">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial;font-size:10.5pt">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial;font-size:10.5pt">
<div dir="ltr" style="margin-top: 20px;">
<div style="font-size: 12pt; font-weight: bold;
font-family: sans-serif; color: #7C7C5F;">Harshal Patil</div>
<div style="font-size: 8pt; font-family: sans-serif;
margin-top: 10px;">
<div><span style="font-weight: bold; color:
#336699;">E-mail: </span><a
moz-do-not-send="true"
href="mailto:harshal.patil@in.ibm.com"
style="color: #555">harshal.patil@in.ibm.com</a></div>
</div>
</div>
</div>
</div>
</div>
<div dir="ltr"> </div>
<div dir="ltr"> </div>
<blockquote data-history-content-modified="1"
data-history-expanded="1" dir="ltr" style="border-left:solid
#aaaaaa 2px; margin-left:5px; padding-left:5px;
direction:ltr; margin-right:0px">----- Original message
-----<br>
From: Aline Manera <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:alinefm@linux.vnet.ibm.com"><alinefm@linux.vnet.ibm.com></a><br>
Sent by: <a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:kimchi-devel-bounces@ovirt.org">kimchi-devel-bounces@ovirt.org</a><br>
To: <a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:harshalp@linux.vnet.ibm.com">harshalp@linux.vnet.ibm.com</a>,
Kimchi Devel <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:kimchi-devel@ovirt.org"><kimchi-devel@ovirt.org></a><br>
Cc:<br>
Subject: Re: [Kimchi-devel] [PATCH] Issue #999: Attach
storage to guest on s390x without libvirt<br>
Date: Sat, Sep 17, 2016 1:03 AM<br>
<div><br>
<font face="Default Monospace,Courier
New,Courier,monospace" size="2">On 09/12/2016 06:55 AM,
<a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:harshalp@linux.vnet.ibm.com">harshalp@linux.vnet.ibm.com</a>
wrote:<br>
> From: Harshal Patil <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:harshalp@linux.vnet.ibm.com"><harshalp@linux.vnet.ibm.com></a><br>
><br>
> This patch enables direct storage attachment to
guests running<br>
> on s390x arch without going through libvirt<br>
><br>
> Signed-off-by: Harshal Patil <a
moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:harshalp@linux.vnet.ibm.com"><harshalp@linux.vnet.ibm.com></a><br>
> ---<br>
> docs/API.md | 3 +++<br>
> i18n.py | 3 +++<br>
> model/vmstorages.py | 25
+++++++++++++++++++++++--<br>
> utils.py | 2 +-<br>
> 4 files changed, 30 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/docs/API.md b/docs/API.md<br>
> index 6678ac5..c9d5d2a 100644<br>
> --- a/docs/API.md<br>
> +++ b/docs/API.md<br>
> @@ -239,6 +239,9 @@ Represents a snapshot of the
Virtual Machine's primary monitor.<br>
> * path: Path of cdrom iso.<br>
> * pool: Storage pool which disk image file
locate in.<br>
> * vol: Storage volume name of disk image.<br>
> + * dir_path: s390x specific attribute to attach
direct storage without libvirt<br>
> + * format: s390x specific attribute specify the
format of direct storage<br>
> + * size: s390x specific attribute to specify
the size of direct storage<br>
><br>
> ### Sub-resource: storage<br>
> **URI:**
/plugins/kimchi/vms/*:name*/storages/*:dev*<br>
> diff --git a/i18n.py b/i18n.py<br>
> index fc7dbc7..ceb4286 100644<br>
> --- a/i18n.py<br>
> +++ b/i18n.py<br>
> @@ -332,6 +332,9 @@ messages = {<br>
> "KCHVMSTOR0016E": _("Volume already in use by
other virtual machine."),<br>
> "KCHVMSTOR0017E": _("Only one of path or
pool/volume can be specified to add a new virtual
machine disk"),<br>
> "KCHVMSTOR0018E": _("Volume chosen with
format %(format)s does not fit in the storage type
%(type)s"),<br>
> + "KCHVMSTOR0019E": _("On s390x arch one of
pool, path of dir_path must be specified"),<br>
> + "KCHVMSTOR0020E": _("On s390x arch 'format'
must be specified while attaching disk to virtual
machine"),<br>
> + "KCHVMSTOR0021E": _("Virtual disk already
exists on the system: %(disk_path)s"),<br>
><br>
> "KCHSNAP0001E": _("Virtual machine '%(vm)s'
must be stopped before creating a snapshot of it."),<br>
> "KCHSNAP0002E": _("Unable to create snapshot
'%(name)s' on virtual machine '%(vm)s'. Details:
%(err)s"),<br>
> diff --git a/model/vmstorages.py
b/model/vmstorages.py<br>
> index 2e9f783..156a16f 100644<br>
> --- a/model/vmstorages.py<br>
> +++ b/model/vmstorages.py<br>
> @@ -17,6 +17,7 @@<br>
> # License along with this library; if not, write
to the Free Software<br>
> # Foundation, Inc., 51 Franklin Street, Fifth
Floor, Boston, MA 02110-1301 USA<br>
><br>
> +import os<br>
> import string<br>
> from lxml import etree<br>
><br>
> @@ -30,6 +31,7 @@ from
wok.plugins.kimchi.model.storagevolumes import
StorageVolumeModel<br>
> from wok.plugins.kimchi.model.utils import
get_vm_config_flag<br>
> from wok.plugins.kimchi.model.vms import
DOM_STATE_MAP, VMModel<br>
> from wok.plugins.kimchi.osinfo import lookup<br>
> +from wok.plugins.kimchi.utils import
create_disk_image, is_s390x<br>
> from wok.plugins.kimchi.xmlutils.disk import
get_device_node, get_disk_xml<br>
> from wok.plugins.kimchi.xmlutils.disk import
get_vm_disk_info, get_vm_disks<br>
><br>
> @@ -82,11 +84,30 @@ class VMStoragesModel(object):<br>
> # Path will never be blank due to
API.json verification.<br>
> # There is no need to cover this case
here.<br>
> if not ('vol' in params) ^ ('path' in
params):<br>
> - raise
InvalidParameter("KCHVMSTOR0017E")<br>
> + if not is_s390x():<br>
> + raise
InvalidParameter("KCHVMSTOR0017E")<br>
> +<br>
<br>
> + if 'dir_path' not in params:<br>
> + raise
InvalidParameter("KCHVMSTOR0019E")<br>
<br>
>The dir_path is exclusively for s390x. You need to
add this verification<br>
>too.</font></div>
</blockquote>
<div dir="ltr"><font face="Default Monospace,Courier
New,Courier,monospace" size="2">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.</font></div>
</div>
</blockquote>
<br>
<br>
Sorry but how the first condition is related to the second one?!<br>
<br>
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).<br>
<br>
So or you add a is_s390x() condition to the second statement or
you place the whole first block in a if conduction:<br>
<br>
if not is390x():<br>
if not (vol in params) ^ (path in params):<br>
(...)<br>
else:<br>
if dir_path not in params:<br>
raise ...<br>
<br>
</blockquote>
Below is the flow:<br>
<br>
1) On x86, if path or volume is not present in params then raise
Invalid parameter(<font size="2">KCHVMSTOR0017E</font>). (Same as it
is in upstream code)<br>
2) On x86 or s390x, if any one of them is present don't raise
invalid parameter(<font size="2">KCHVMSTOR0017E</font>).. (Same as
it is in upstream code)<br>
3) On s390x, if path or volume is not present instead of raising
invalid parameter, it should go and check for dir_path and if
dir_path is also not present then only raise Invalid parameter(<font
size="2">KCHVMSTOR0019E</font>).<br>
<br>
So on x86, if I have specified a vol or a path then it will not go
inside first if itself(<font size="2">not ('vol' in params) ^
('path' in params) will be false). <br>
and on x86, if I have neither vol nor path then it will go inside
first if and also second one and raise invalid parameter.<br>
<br>
And on x86, due to first nested 'if' (that is </font><font
size="2"><font size="2">not is_s390x()) it will never go to second
nested 'if' (</font></font><font size="2"><font size="2"><font
size="2">'dir_path' not in params).<br>
<br>
In Summary, on x86 if I have neither vol nor path then it
should raise exception </font></font></font><font size="2"><font
size="2"><font size="2"><font size="2">KCHVMSTOR0017E </font>(same
as currently upstream code behaves) and does not proceed
further.<br>
and on s390x we should check for vol and path, and if non of
it is present then instead of raising exception </font></font></font><font
size="2"><font size="2"><font size="2"><font size="2">KCHVMSTOR0017E
go ahead and </font>check for addition key dir_path. If
dir_path is also not present then only raise exception </font></font></font><font
size="2"><font size="2"><font size="2"><font size="2">KCHVMSTOR0019E</font>.<br>
<br>
Hope I make sense to you.<br>
</font></font></font><font face="Default Monospace,Courier
New,Courier,monospace" size="2"><br>
</font>
<blockquote
cite="mid:31ba4c43-9a32-ee62-8723-26050fdebc48@linux.vnet.ibm.com"
type="cite">
<blockquote
cite="mid:OF5727B727.96B7C675-ON00258033.0054FCC7-00258033.005575D0@notes.na.collabserv.com"
type="cite">
<div class="socmaildefaultfont" dir="ltr"
style="font-family:Arial;font-size:10.5pt">
<blockquote class="history-quote-1474298873569"
data-history-content-modified="1" data-history-expanded="1"
dir="ltr" style="border-left:solid #aaaaaa 2px;
margin-left:5px; padding-left:5px; direction:ltr;
margin-right:0px">
<div> </div>
<div><br>
<br>
<font face="Default Monospace,Courier
New,Courier,monospace" size="2">> dom =
VMModel.get_vm(vm_name, self.conn)<br>
> params['bus'] =
_get_device_bus(params['type'], dom)<br>
> - params['format'] = 'raw'<br>
> +<br>
> + if is_s390x() and params['type'] ==
'disk':<br>
> + if 'format' not in params:<br>
> + raise
InvalidParameter("KCHVMSTOR0020E")<br>
> + if 'dir_path' in params:<br>
> + size = params['size']<br>
> + name = params['name']<br>
> + dir_path = params.get('dir_path')<br>
> + params['path'] = dir_path + "/" +
name<br>
> + if os.path.exists(params['path']):<br>
> + raise
InvalidParameter("KCHVMSTOR0021E",<br>
> +
{'disk_path': params['path']})<br>
> +
create_disk_image(format_type=params['format'],<br>
> +
path=params['path'], capacity=size)<br>
> + else:<br>
> + params['format'] = 'raw'<br>
><br>
> dev_list = [dev for dev, bus in
get_vm_disks(dom).iteritems()<br>
> if bus == params['bus']]<br>
> diff --git a/utils.py b/utils.py<br>
> index 0fca191..7fdec02 100644<br>
> --- a/utils.py<br>
> +++ b/utils.py<br>
> @@ -305,4 +305,4 @@ def
create_disk_image(format_type, path, capacity):<br>
> if rc != 0:<br>
> raise OperationFailed("KCHTMPL0035E",
{'err': err})<br>
><br>
> - return<br>
> \ No newline at end of file<br>
> + return<br>
<br>
_______________________________________________<br>
Kimchi-devel mailing list<br>
<a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:Kimchi-devel@ovirt.org">Kimchi-devel@ovirt.org</a><br>
<a moz-do-not-send="true"
href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel"
target="_blank">http://lists.ovirt.org/mailman/listinfo/kimchi-devel</a></font><br>
</div>
</blockquote>
<div dir="ltr"> </div>
</div>
<br>
</blockquote>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
Kimchi-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Kimchi-devel@ovirt.org">Kimchi-devel@ovirt.org</a>
<a class="moz-txt-link-freetext" href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel">http://lists.ovirt.org/mailman/listinfo/kimchi-devel</a>
</pre>
</blockquote>
<br>
</body>
</html>