rposudnevskiy / RBDSR

RBDSR - XenServer/XCP-ng Storage Manager plugin for CEPH
GNU Lesser General Public License v2.1
58 stars 23 forks source link

a little bugs #25

Closed suzj801 closed 7 years ago

suzj801 commented 7 years ago
diff --git a/RBDSR.py b/RBDSR.py
index f310172..b0fe479 100644
--- a/RBDSR.py
+++ b/RBDSR.py
@@ -708,8 +708,8 @@ class RBDVDI(VDI.VDI, cephutils.VDI):
             elif self.mode == "nbd":
                 self._disable_rbd_caching()
                 cmdout = util.pread2(["rbd-nbd", "--nbds_max", str(cephutils.NBDS_MAX), "-c", "/etc/ceph/ceph.conf.nocaching", "map", "%s/%s" % (self.sr.CEPH_POOL_NAME, _vdi_name), "--name", self.sr.CEPH_USER]).rstrip('\n')
-                util.pread2(["ln", "-s", cmdout, _dev_name])
-            util.pread2(["ln", "-s", cmdout, dev_name])
+                util.pread2(["ln", "-bs", cmdout, _dev_name])
+            util.pread2(["ln", "-bs", cmdout, dev_name])

             self.path = self.sr._get_path(vdi_uuid)
             if not util.pathexists(self.path):
diff --git a/ceph_plugin.py b/ceph_plugin.py
index 09ccdde..239a1f0 100644
--- a/ceph_plugin.py
+++ b/ceph_plugin.py
@@ -79,7 +79,7 @@ def _map(session, arg_dict):
             dev = util.pread2(["rbd-nbd", "--nbds_max", NBDS_MAX, "-c", "/etc/ceph/ceph.conf.nocaching", "map", "%s/%s" % (CEPH_POOL_NAME, _vdi_name), "--name", CEPH_USER]).rstrip('\n')
         else:
             dev = util.pread2(["rbd-nbd", "--nbds_max", NBDS_MAX, "map", "%s/%s" % (CEPH_POOL_NAME, _vdi_name), "--name", CEPH_USER]).rstrip('\n')
-        util.pread2(["ln", "-s", dev, _dev_name])
+        util.pread2(["ln", "-bs", dev, _dev_name])

     if dm == "linear":
         util.pread2(["dmsetup", "create", _dm_name, "--table", "0 %s linear %s 0" % (str(int(size) / 512), dev)])
diff --git a/cephutils.py b/cephutils.py
index 221fae2..2493a84 100644
--- a/cephutils.py
+++ b/cephutils.py
@@ -506,6 +506,7 @@ class VDI:
                 "dm":dm,
                 "size":str(size)}
         self._call_plugin('map',args)
+        self.session.xenapi.VDI.remove_from_sm_config(vdi_ref, 'dm')
         self.session.xenapi.VDI.add_to_sm_config(vdi_ref, 'dm', dm)

     def _unmap_VHD(self, vdi_uuid, size):
@@ -571,6 +572,7 @@ class VDI:
                 "dm":dm,
                 "size":str(size)}
         self._call_plugin('map',args)
+        self.session.xenapi.VDI.remove_from_sm_config(vdi_ref, 'dm')
         self.session.xenapi.VDI.add_to_sm_config(snap_ref, 'dm', dm)

     def _unmap_SNAP(self, vdi_uuid, snap_uuid, size):
@@ -636,6 +638,7 @@ class VDI:
                 "dm":dm,
                 "size":str(size)}
         self._call_plugin('map',args)
+        self.session.xenapi.VDI.remove_from_sm_config(vdi_ref, 'dm')
         self.session.xenapi.VDI.add_to_sm_config(vdi_ref, 'dm', dm)

     def _unmap_sxm_mirror(self, vdi_uuid, size):
@@ -694,6 +697,7 @@ class VDI:
                 "dm":dm,
                 "size":str(size)}
         self._call_plugin('map',args)
+        self.session.xenapi.VDI.remove_from_sm_config(vdi_ref, 'dm')
         self.session.xenapi.VDI.add_to_sm_config(vdi_ref, 'dm', dm)

     def _unmap_sxm_base(self, vdi_uuid, size):
@@ -774,4 +778,4 @@ class VDI:
         self._map_VHD(mirror_uuid, size, "linear")
         #---
         if not blktap2.VDI.tap_unpause(self.session, self.sr.uuid, mirror_uuid, None):
-            raise util.SMException("failed to unpause VDI %s" % mirror_uuid)
\ No newline at end of file
+            raise util.SMException("failed to unpause VDI %s" % mirror_uuid)
rposudnevskiy commented 7 years ago

Hi, Thank you for the interest to the project. A few comments on your changes.

Regarding this changes - util.pread2(["ln", "-s", dev, _dev_name]) + util.pread2(["ln", "-bs", dev, _dev_name]) If all is normal, the link that is made here shouldn't exist, as it should be deleted on unmap operation. If it exists then something went wrong. I think it would be better to receive an error and make some investigation than ignore the problem by making the backup copy of existing file using -b option with ln command

The same situation with this changes + self.session.xenapi.VDI.remove_from_sm_config(vdi_ref, 'dm') self.session.xenapi.VDI.add_to_sm_config(vdi_ref, 'dm', dm) The dm attribute in sm_config is deleted on unmap operation and shouldn't exist before VDI is mapped. So it is not required to delete it before add it to sm_config if all is normal. And again it would be better to receive error than ignore problem. If VDI is not mapped but sm_config has atttribute dm then something went wrong and some investigation is required.

Thank you.

suzj801 commented 7 years ago

Thanks for reply. This may happend after the host crushed or power off. So I find the problem that can't boot the guest vms.