<div class="socmaildefaultfont" dir="ltr" style="font-family:Arial;font-size:10.5pt" ><div dir="ltr" >&nbsp;</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&nbsp;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 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" >&nbsp;</div>
<div dir="ltr" >&nbsp;</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 &lt;alinefm@linux.vnet.ibm.com&gt;<br>Sent by: kimchi-devel-bounces@ovirt.org<br>To: harshalp@linux.vnet.ibm.com, Kimchi Devel &lt;kimchi-devel@ovirt.org&gt;<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>&nbsp;
<div><br><font face="Default Monospace,Courier New,Courier,monospace" size="2" >On 09/12/2016 06:55 AM, harshalp@linux.vnet.ibm.com wrote:<br>&gt; From: Harshal Patil &lt;harshalp@linux.vnet.ibm.com&gt;<br>&gt;<br>&gt; This patch enables direct storage attachment to guests running<br>&gt; on s390x arch without going through libvirt<br>&gt;<br>&gt; Signed-off-by: Harshal Patil &lt;harshalp@linux.vnet.ibm.com&gt;<br>&gt; ---<br>&gt; &nbsp; docs/API.md &nbsp; &nbsp; &nbsp; &nbsp; | &nbsp;3 +++<br>&gt; &nbsp; i18n.py &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; | &nbsp;3 +++<br>&gt; &nbsp; model/vmstorages.py | 25 +++++++++++++++++++++++--<br>&gt; &nbsp; utils.py &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;| &nbsp;2 +-<br>&gt; &nbsp; 4 files changed, 30 insertions(+), 3 deletions(-)<br>&gt;<br>&gt; diff --git a/docs/API.md b/docs/API.md<br>&gt; index 6678ac5..c9d5d2a 100644<br>&gt; --- a/docs/API.md<br>&gt; +++ b/docs/API.md<br>&gt; @@ -239,6 +239,9 @@ Represents a snapshot of the Virtual Machine's primary monitor.<br>&gt; &nbsp; &nbsp; &nbsp; * path: Path of cdrom iso.<br>&gt; &nbsp; &nbsp; &nbsp; * pool: Storage pool which disk image file locate in.<br>&gt; &nbsp; &nbsp; &nbsp; * vol: Storage volume name of disk image.<br>&gt; + &nbsp; &nbsp;* dir_path: s390x specific attribute to attach direct storage without libvirt<br>&gt; + &nbsp; &nbsp;* format: s390x specific attribute specify the format of direct storage<br>&gt; + &nbsp; &nbsp;* size: s390x specific attribute to specify the size of direct storage<br>&gt;<br>&gt; &nbsp; ### Sub-resource: storage<br>&gt; &nbsp; **URI:** /plugins/kimchi/vms/*:name*/storages/*:dev*<br>&gt; diff --git a/i18n.py b/i18n.py<br>&gt; index fc7dbc7..ceb4286 100644<br>&gt; --- a/i18n.py<br>&gt; +++ b/i18n.py<br>&gt; @@ -332,6 +332,9 @@ messages = {<br>&gt; &nbsp; &nbsp; &nbsp; "KCHVMSTOR0016E": _("Volume already in use by other virtual machine."),<br>&gt; &nbsp; &nbsp; &nbsp; "KCHVMSTOR0017E": _("Only one of path or pool/volume can be specified to add a new virtual machine disk"),<br>&gt; &nbsp; &nbsp; &nbsp; "KCHVMSTOR0018E": _("Volume chosen with format %(format)s does not fit in the storage type %(type)s"),<br>&gt; + &nbsp; &nbsp;"KCHVMSTOR0019E": _("On s390x arch one of pool, path of dir_path must be specified"),<br>&gt; + &nbsp; &nbsp;"KCHVMSTOR0020E": _("On s390x arch 'format' must be specified while attaching disk to virtual machine"),<br>&gt; + &nbsp; &nbsp;"KCHVMSTOR0021E": _("Virtual disk already exists on the system: %(disk_path)s"),<br>&gt;<br>&gt; &nbsp; &nbsp; &nbsp; "KCHSNAP0001E": _("Virtual machine '%(vm)s' must be stopped before creating a snapshot of it."),<br>&gt; &nbsp; &nbsp; &nbsp; "KCHSNAP0002E": _("Unable to create snapshot '%(name)s' on virtual machine '%(vm)s'. Details: %(err)s"),<br>&gt; diff --git a/model/vmstorages.py b/model/vmstorages.py<br>&gt; index 2e9f783..156a16f 100644<br>&gt; --- a/model/vmstorages.py<br>&gt; +++ b/model/vmstorages.py<br>&gt; @@ -17,6 +17,7 @@<br>&gt; &nbsp; # License along with this library; if not, write to the Free Software<br>&gt; &nbsp; # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA &nbsp;02110-1301 USA<br>&gt;<br>&gt; +import os<br>&gt; &nbsp; import string<br>&gt; &nbsp; from lxml import etree<br>&gt;<br>&gt; @@ -30,6 +31,7 @@ from wok.plugins.kimchi.model.storagevolumes import StorageVolumeModel<br>&gt; &nbsp; from wok.plugins.kimchi.model.utils import get_vm_config_flag<br>&gt; &nbsp; from wok.plugins.kimchi.model.vms import DOM_STATE_MAP, VMModel<br>&gt; &nbsp; from wok.plugins.kimchi.osinfo import lookup<br>&gt; +from wok.plugins.kimchi.utils import create_disk_image, is_s390x<br>&gt; &nbsp; from wok.plugins.kimchi.xmlutils.disk import get_device_node, get_disk_xml<br>&gt; &nbsp; from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks<br>&gt;<br>&gt; @@ -82,11 +84,30 @@ class VMStoragesModel(object):<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; # Path will never be blank due to API.json verification.<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; # There is no need to cover this case here.<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if not ('vol' in params) ^ ('path' in params):<br>&gt; - &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise InvalidParameter("KCHVMSTOR0017E")<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if not is_s390x():<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise InvalidParameter("KCHVMSTOR0017E")<br>&gt; +<br><br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if 'dir_path' not in params:<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise InvalidParameter("KCHVMSTOR0019E")<br><br>&gt;The dir_path is exclusively for s390x. You need to add this verification<br>&gt;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&nbsp;s390x it will enter the code path&nbsp;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>
<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>&nbsp;</div>
<div><br><br><font face="Default Monospace,Courier New,Courier,monospace" size="2" >&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; dom = VMModel.get_vm(vm_name, self.conn)<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; params['bus'] = _get_device_bus(params['type'], dom)<br>&gt; - &nbsp; &nbsp; &nbsp; &nbsp;params['format'] = 'raw'<br>&gt; +<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp;if is_s390x() and params['type'] == 'disk':<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if 'format' not in params:<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise InvalidParameter("KCHVMSTOR0020E")<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if 'dir_path' in params:<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;size = params['size']<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;name = params['name']<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;dir_path = params.get('dir_path')<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;params['path'] = dir_path + "/" + name<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if os.path.exists(params['path']):<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;raise InvalidParameter("KCHVMSTOR0021E",<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; {'disk_path': params['path']})<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;create_disk_image(format_type=params['format'],<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;path=params['path'], capacity=size)<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp;else:<br>&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;params['format'] = 'raw'<br>&gt;<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; dev_list = [dev for dev, bus in get_vm_disks(dom).iteritems()<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if bus == params['bus']]<br>&gt; diff --git a/utils.py b/utils.py<br>&gt; index 0fca191..7fdec02 100644<br>&gt; --- a/utils.py<br>&gt; +++ b/utils.py<br>&gt; @@ -305,4 +305,4 @@ def create_disk_image(format_type, path, capacity):<br>&gt; &nbsp; &nbsp; &nbsp; if rc != 0:<br>&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; raise OperationFailed("KCHTMPL0035E", {'err': err})<br>&gt;<br>&gt; - &nbsp; &nbsp;return<br>&gt; \ No newline at end of file<br>&gt; + &nbsp; &nbsp;return<br><br>_______________________________________________<br>Kimchi-devel mailing list<br>Kimchi-devel@ovirt.org<br><a href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel" target="_blank" >http://lists.ovirt.org/mailman/listinfo/kimchi-devel</a></font><br>&nbsp;</div></blockquote>
<div dir="ltr" >&nbsp;</div></div><BR>