Merge remote-tracking branch 'gh/wip-6047'

Reviewed-by: Sage Weil <sage@inktank.com>

qa/workunits/mon/rbd_snaps_ops.sh

@@ -0,0 +1,39 @@
+#!/bin/bash
+
+# attempt to trigger #6047
+
+
+cmd_no=0
+expect()
+{
+  cmd_no=$(($cmd_no+1))
+  cmd="$1"
+  expected=$2
+  echo "[$cmd_no] $cmd"
+  eval  $cmd
+  ret=$?
+  if [[ $ret -ne $expected ]]; then
+    echo "[$cmd_no] unexpected return '$ret', expected '$expected'"
+    exit 1
+  fi
+}
+
+expect 'ceph osd pool create test 256 256' 0
+expect 'ceph osd pool mksnap test snapshot' 0
+expect 'ceph osd pool rmsnap test snapshot' 0
+
+expect 'rbd --pool=test create --size=102400 image' 0
+expect 'rbd --pool=test snap create image@snapshot' 22
+
+expect 'ceph osd pool delete test test --yes-i-really-really-mean-it' 0
+expect 'ceph osd pool create test 256 256' 0
+expect 'rbd --pool=test create --size=102400 image' 0
+expect 'rbd --pool=test snap create image@snapshot' 0
+expect 'rbd --pool=test snap ls image' 0
+expect 'rbd --pool=test snap rm image@snapshot' 0
+
+expect 'ceph osd pool mksnap test snapshot' 22
+
+expect 'ceph osd pool delete test test --yes-i-really-really-mean-it' 0
+
+echo OK

src/mon/OSDMonitor.cc

@@ -3381,7 +3381,11 @@ done:
     string snapname;
     cmd_getval(g_ceph_context, cmdmap, "snap", snapname);
     const pg_pool_t *p = osdmap.get_pg_pool(pool);
-    if (p->snap_exists(snapname.c_str())) {
+    if (p->is_unmanaged_snaps_mode()) {
+      ss << "pool " << poolstr << " is in unmanaged snaps mode";
+      err = -EINVAL;
+      goto reply;
+    } else if (p->snap_exists(snapname.c_str())) {
       ss << "pool " << poolstr << " snap " << snapname << " already exists";
       err = 0;
       goto reply;
@@ -3415,7 +3419,11 @@ done:
     string snapname;
     cmd_getval(g_ceph_context, cmdmap, "snap", snapname);
     const pg_pool_t *p = osdmap.get_pg_pool(pool);
-    if (!p->snap_exists(snapname.c_str())) {
+    if (p->is_unmanaged_snaps_mode()) {
+      ss << "pool " << poolstr << " is in unmanaged snaps mode";
+      err = -EINVAL;
+      goto reply;
+    } else if (!p->snap_exists(snapname.c_str())) {
       ss << "pool " << poolstr << " snap " << snapname << " does not exist";
       err = 0;
       goto reply;
@@ -3866,7 +3874,7 @@ bool OSDMonitor::preprocess_pool_op(MPoolOp *m)
     _pool_op_reply(m, 0, osdmap.get_epoch());
     return true;
   }
-  
+
   // check if the snap and snapname exists
   bool snap_exists = false;
   const pg_pool_t *p = osdmap.get_pg_pool(m->pool);
@@ -3962,6 +3970,38 @@ bool OSDMonitor::prepare_pool_op(MPoolOp *m)
   int ret = 0;
   bool changed = false;
 
+  if (!osdmap.have_pg_pool(m->pool)) {
+    _pool_op_reply(m, -ENOENT, osdmap.get_epoch());
+    return false;
+  }
+
+  const pg_pool_t *pool = osdmap.get_pg_pool(m->pool);
+
+  switch (m->op) {
+    case POOL_OP_CREATE_SNAP:
+    case POOL_OP_DELETE_SNAP:
+      if (!pool->is_unmanaged_snaps_mode()) {
+        bool snap_exists = pool->snap_exists(m->name.c_str());
+        if ((m->op == POOL_OP_CREATE_SNAP && snap_exists)
+          || (m->op == POOL_OP_CREATE_SNAP && !snap_exists)) {
+          ret = 0;
+        } else {
+          break;
+        }
+      } else {
+        ret = -EINVAL;
+      }
+      _pool_op_reply(m, ret, osdmap.get_epoch());
+      return false;
+
+    case POOL_OP_CREATE_UNMANAGED_SNAP:
+    case POOL_OP_DELETE_UNMANAGED_SNAP:
+      if (pool->is_pool_snaps_mode()) {
+        _pool_op_reply(m, -EINVAL, osdmap.get_epoch());
+        return false;
+      }
+  }
+
   // projected pool info
   pg_pool_t pp;
   if (pending_inc.new_pools.count(m->pool))
@@ -3988,7 +4028,7 @@ bool OSDMonitor::prepare_pool_op(MPoolOp *m)
       goto out;
     }
   }
- 
+
   switch (m->op) {
   case POOL_OP_CREATE_SNAP:
     if (!pp.snap_exists(m->name.c_str())) {
@@ -4043,8 +4083,7 @@ bool OSDMonitor::prepare_pool_op(MPoolOp *m)
 
  out:
   wait_for_finished_proposal(new OSDMonitor::C_PoolOp(this, m, ret, pending_inc.epoch, &reply_data));
-  propose_pending();
-  return false;
+  return true;
 }
 
 bool OSDMonitor::prepare_pool_op_create(MPoolOp *m)

src/rbd.cc

@@ -2484,7 +2484,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (r < 0) {
       cerr << "rbd: error opening pool " << poolname << ": "
 	   << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
   }
 
@@ -2511,7 +2511,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (r < 0) {
       cerr << "rbd: error opening image " << imgname << ": "
 	   << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
   }
 
@@ -2526,7 +2526,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (r < 0) {
       cerr << "rbd: error setting snapshot context: " << cpp_strerror(-r)
 	   << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
   }
 
@@ -2535,14 +2535,14 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (r < 0) {
       cerr << "rbd: error opening pool " << dest_poolname << ": "
 	   << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
   }
 
   if (opt_cmd == OPT_CREATE || opt_cmd == OPT_RESIZE) {
     if (!size_set) {
       cerr << "rbd: must specify --size <MB>" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
   }
 
@@ -2558,7 +2558,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
       default:
         cerr << "rbd: list: " << cpp_strerror(-r) << std::endl;
       }
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2566,19 +2566,19 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (order && (order < 12 || order > 25)) {
       cerr << "rbd: order must be between 12 (4 KB) and 25 (32 MB)"
 	   << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     if ((stripe_unit && !stripe_count) || (!stripe_unit && stripe_count)) {
       cerr << "must specify both (or neither) of stripe-unit and stripe-count"
 	   << std::endl;
       usage();
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_create(rbd, io_ctx, imgname, size, &order, format, features,
 		  stripe_unit, stripe_count);
     if (r < 0) {
       cerr << "rbd: create error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2586,14 +2586,14 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (order && (order < 12 || order > 25)) {
       cerr << "rbd: order must be between 12 (4 KB) and 25 (32 MB)"
 	   << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
 
     r = do_clone(rbd, io_ctx, imgname, snapname, dest_io_ctx, destname,
 		 features, &order);
     if (r < 0) {
       cerr << "rbd: clone error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2601,7 +2601,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_flatten(image);
     if (r < 0) {
       cerr << "rbd: flatten error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2609,7 +2609,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_rename(rbd, io_ctx, imgname, destname);
     if (r < 0) {
       cerr << "rbd: rename error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2617,7 +2617,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_show_info(imgname, image, snapname, formatter.get());
     if (r < 0) {
       cerr << "rbd: info: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2647,18 +2647,18 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = image.stat(info, sizeof(info));
     if (r < 0) {
       cerr << "rbd: resize error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
 
     if (info.size > size && !resize_allow_shrink) {
       cerr << "rbd: shrinking an image is only allowed with the --allow-shrink flag" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
 
     r = do_resize(image, size);
     if (r < 0) {
       cerr << "rbd: resize error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2671,87 +2671,87 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     if (r < 0) {
       cerr << "rbd: failed to list snapshots: " << cpp_strerror(-r)
 	   << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_SNAP_CREATE:
     if (!imgname || !snapname) {
       cerr << "rbd: snap create requires image and snapname" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_add_snap(image, snapname);
     if (r < 0) {
       cerr << "rbd: failed to create snapshot: " << cpp_strerror(-r)
 	   << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_SNAP_ROLLBACK:
     if (!imgname) {
       cerr << "rbd: snap rollback requires image name" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_rollback_snap(image, snapname);
     if (r < 0) {
       cerr << "rbd: rollback failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_SNAP_REMOVE:
     if (!imgname) {
       cerr << "rbd: snap remove requires image name" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_remove_snap(image, snapname);
-    if (r == -EBUSY) {
-      cerr << "rbd: snapshot '" << snapname << "' is protected from removal."
-	   << std::endl;
-      return EXIT_FAILURE;
-    }
     if (r < 0) {
-      cerr << "rbd: failed to remove snapshot: " << cpp_strerror(-r)
-	   << std::endl;
-      return EXIT_FAILURE;
+      if (r == -EBUSY) {
+        cerr << "rbd: snapshot '" << snapname << "' is protected from removal."
+             << std::endl;
+      } else {
+        cerr << "rbd: failed to remove snapshot: " << cpp_strerror(-r)
+             << std::endl;
+      }
+      return -r;
     }
     break;
 
   case OPT_SNAP_PURGE:
     if (!imgname) {
       cerr << "rbd: snap purge requires image name" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_purge_snaps(image);
     if (r < 0) {
       cerr << "rbd: removing snaps failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_SNAP_PROTECT:
     if (!imgname) {
       cerr << "rbd: snap protect requires image name" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_protect_snap(image, snapname);
     if (r < 0) {
       cerr << "rbd: protecting snap failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_SNAP_UNPROTECT:
     if (!imgname) {
       cerr << "rbd: snap unprotect requires image name" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_unprotect_snap(image, snapname);
     if (r < 0) {
       cerr << "rbd: unprotecting snap failed: " << cpp_strerror(-r)
 	   << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2759,19 +2759,19 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_list_children(image, formatter.get());
     if (r < 0) {
       cerr << "rbd: listing children failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_EXPORT:
     if (!path) {
       cerr << "rbd: export requires pathname" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_export(image, path);
     if (r < 0) {
       cerr << "rbd: export error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2779,32 +2779,32 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_diff(image, fromsnapname, formatter.get());
     if (r < 0) {
       cerr << "rbd: diff error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_EXPORT_DIFF:
     if (!path) {
       cerr << "rbd: export-diff requires pathname" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_export_diff(image, fromsnapname, snapname, path);
     if (r < 0) {
       cerr << "rbd: export-diff error: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
   case OPT_IMPORT:
     if (!path) {
       cerr << "rbd: import requires pathname" << std::endl;
-      return EXIT_FAILURE;
+      return EINVAL;
     }
     r = do_import(rbd, dest_io_ctx, destname, &order, path,
 		  format, features, size);
     if (r < 0) {
       cerr << "rbd: import failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2813,7 +2813,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_import_diff(image, path);
     if (r < 0) {
       cerr << "rbd: import-diff failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2821,7 +2821,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_copy(image, dest_io_ctx, destname);
     if (r < 0) {
       cerr << "rbd: copy failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2829,7 +2829,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_watch(io_ctx, imgname);
     if (r < 0) {
       cerr << "rbd: watch failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2837,7 +2837,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_kernel_add(poolname, imgname, snapname);
     if (r < 0) {
       cerr << "rbd: add failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2845,7 +2845,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_kernel_rm(devpath);
     if (r < 0) {
       cerr << "rbd: remove failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2853,7 +2853,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_kernel_showmapped(formatter.get());
     if (r < 0) {
       cerr << "rbd: showmapped failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2861,7 +2861,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_lock_list(image, formatter.get());
     if (r < 0) {
       cerr << "rbd: listing locks failed: " << cpp_strerror(r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2878,7 +2878,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
       } else {
 	cerr << "rbd: taking lock failed: " << cpp_strerror(r) << std::endl;
       }
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2886,7 +2886,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_lock_remove(image, lock_cookie, lock_client);
     if (r < 0) {
       cerr << "rbd: releasing lock failed: " << cpp_strerror(r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
 
@@ -2894,7 +2894,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
     r = do_bench_write(image, bench_io_size, bench_io_threads, bench_bytes, bench_pattern);
     if (r < 0) {
       cerr << "bench-write failed: " << cpp_strerror(-r) << std::endl;
-      return EXIT_FAILURE;
+      return -r;
     }
     break;
   }