<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <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 class="moz-txt-link-rfc2396E" href="mailto:alinefm@linux.vnet.ibm.com">&lt;alinefm@linux.vnet.ibm.com&gt;</a><br>
          Sent by: <a class="moz-txt-link-abbreviated" href="mailto:kimchi-devel-bounces@ovirt.org">kimchi-devel-bounces@ovirt.org</a><br>
          To: <a class="moz-txt-link-abbreviated" href="mailto:harshalp@linux.vnet.ibm.com">harshalp@linux.vnet.ibm.com</a>, Kimchi Devel
          <a class="moz-txt-link-rfc2396E" href="mailto:kimchi-devel@ovirt.org">&lt;kimchi-devel@ovirt.org&gt;</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 class="moz-txt-link-abbreviated" href="mailto:harshalp@linux.vnet.ibm.com">harshalp@linux.vnet.ibm.com</a> wrote:<br>
              &gt; From: Harshal Patil
              <a class="moz-txt-link-rfc2396E" href="mailto:harshalp@linux.vnet.ibm.com">&lt;harshalp@linux.vnet.ibm.com&gt;</a><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
              <a class="moz-txt-link-rfc2396E" href="mailto:harshalp@linux.vnet.ibm.com">&lt;harshalp@linux.vnet.ibm.com&gt;</a><br>
              &gt; ---<br>
              &gt;   docs/API.md         |  3 +++<br>
              &gt;   i18n.py             |  3 +++<br>
              &gt;   model/vmstorages.py | 25 +++++++++++++++++++++++--<br>
              &gt;   utils.py            |  2 +-<br>
              &gt;   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;       * path: Path of cdrom iso.<br>
              &gt;       * pool: Storage pool which disk image file
              locate in.<br>
              &gt;       * vol: Storage volume name of disk image.<br>
              &gt; +    * dir_path: s390x specific attribute to attach
              direct storage without libvirt<br>
              &gt; +    * format: s390x specific attribute specify the
              format of direct storage<br>
              &gt; +    * size: s390x specific attribute to specify the
              size of direct storage<br>
              &gt;<br>
              &gt;   ### Sub-resource: storage<br>
              &gt;   **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;       "KCHVMSTOR0016E": _("Volume already in use by
              other virtual machine."),<br>
              &gt;       "KCHVMSTOR0017E": _("Only one of path or
              pool/volume can be specified to add a new virtual machine
              disk"),<br>
              &gt;       "KCHVMSTOR0018E": _("Volume chosen with format
              %(format)s does not fit in the storage type %(type)s"),<br>
              &gt; +    "KCHVMSTOR0019E": _("On s390x arch one of pool,
              path of dir_path must be specified"),<br>
              &gt; +    "KCHVMSTOR0020E": _("On s390x arch 'format' must
              be specified while attaching disk to virtual machine"),<br>
              &gt; +    "KCHVMSTOR0021E": _("Virtual disk already exists
              on the system: %(disk_path)s"),<br>
              &gt;<br>
              &gt;       "KCHSNAP0001E": _("Virtual machine '%(vm)s'
              must be stopped before creating a snapshot of it."),<br>
              &gt;       "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;   # License along with this library; if not, write to
              the Free Software<br>
              &gt;   # Foundation, Inc., 51 Franklin Street, Fifth
              Floor, Boston, MA  02110-1301 USA<br>
              &gt;<br>
              &gt; +import os<br>
              &gt;   import string<br>
              &gt;   from lxml import etree<br>
              &gt;<br>
              &gt; @@ -30,6 +31,7 @@ from
              wok.plugins.kimchi.model.storagevolumes import
              StorageVolumeModel<br>
              &gt;   from wok.plugins.kimchi.model.utils import
              get_vm_config_flag<br>
              &gt;   from wok.plugins.kimchi.model.vms import
              DOM_STATE_MAP, VMModel<br>
              &gt;   from wok.plugins.kimchi.osinfo import lookup<br>
              &gt; +from wok.plugins.kimchi.utils import
              create_disk_image, is_s390x<br>
              &gt;   from wok.plugins.kimchi.xmlutils.disk import
              get_device_node, get_disk_xml<br>
              &gt;   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;           # Path will never be blank due to API.json
              verification.<br>
              &gt;           # There is no need to cover this case here.<br>
              &gt;           if not ('vol' in params) ^ ('path' in
              params):<br>
              &gt; -            raise InvalidParameter("KCHVMSTOR0017E")<br>
              &gt; +            if not is_s390x():<br>
              &gt; +                raise
              InvalidParameter("KCHVMSTOR0017E")<br>
              &gt; +<br>
              <br>
              &gt; +            if 'dir_path' not in params:<br>
              &gt; +                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 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>
    <br>
    <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">&gt;           dom = VMModel.get_vm(vm_name,
              self.conn)<br>
              &gt;           params['bus'] =
              _get_device_bus(params['type'], dom)<br>
              &gt; -        params['format'] = 'raw'<br>
              &gt; +<br>
              &gt; +        if is_s390x() and params['type'] == 'disk':<br>
              &gt; +            if 'format' not in params:<br>
              &gt; +                raise
              InvalidParameter("KCHVMSTOR0020E")<br>
              &gt; +            if 'dir_path' in params:<br>
              &gt; +                size = params['size']<br>
              &gt; +                name = params['name']<br>
              &gt; +                dir_path = params.get('dir_path')<br>
              &gt; +                params['path'] = dir_path + "/" +
              name<br>
              &gt; +                if os.path.exists(params['path']):<br>
              &gt; +                    raise
              InvalidParameter("KCHVMSTOR0021E",<br>
              &gt; +                                          
              {'disk_path': params['path']})<br>
              &gt; +              
               create_disk_image(format_type=params['format'],<br>
              &gt; +                                
               path=params['path'], capacity=size)<br>
              &gt; +        else:<br>
              &gt; +            params['format'] = 'raw'<br>
              &gt;<br>
              &gt;           dev_list = [dev for dev, bus in
              get_vm_disks(dom).iteritems()<br>
              &gt;                       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;       if rc != 0:<br>
              &gt;           raise OperationFailed("KCHTMPL0035E",
              {'err': err})<br>
              &gt;<br>
              &gt; -    return<br>
              &gt; \ No newline at end of file<br>
              &gt; +    return<br>
              <br>
              _______________________________________________<br>
              Kimchi-devel mailing list<br>
              <a 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>
  </body>
</html>