[OpenIndiana-discuss] Fwd: 2nd round - Bug 1204 zoneadm cannot create clone of zone from snapshot

[Cedric Blancher]

Repost
---------- Forwarded message ----------
From: Cedric Blancher <cedric.blancher at gmail.com>
Date: 15 November 2013 00:17
Subject: Re: [OpenIndiana-discuss] 2nd round - Bug 1204 zoneadm cannot
create clone of zone from snapshot
To: Discussion list for OpenIndiana <openindiana-discuss at openindiana.org>


On 15 November 2013 00:07, Jim Klimov <jimklimov at cos.ru> wrote:
> Hello all,
>
> Second round with a more complete fix... might become my first RTI soon
> if it gains enough "yay"'s ;)
>
> https://www.illumos.org/issues/1204
> https://www.illumos.org/attachments/download/1027/ips-clone.patch
>
> Updated the patch. Now it supports cloning from a different zone boot
> environment of the same zone by accepting the following values for "-s"
> (snapshot):
> 1) *@* - such strings are interpreted as dataset at snapname, including an
>    empty dataset part (only "@snapname" is then accepted for currently
>    active source ZBE)
> 2) */* - such strings specify the dataset, of which a snapshot will be
>    made and used automatically
> 3) * - any other strings specify the snapshot name, which should exist
>    of the currently active source zone root
>
> Sanity checks are done that the specified source dataset (if specified)
> conforms to the naming and attributes of ZBE's, and is a ZBE of the
> specified source zone, and that a snapshot (if specified) exists in the
> selected source dataset.
>
> Also added is a "-X" 'brand-specific argument' which allows to disable
> "sys-unconfig" in the newly cloned zone. Sometimes admins and their
> carefully prepared template zones know better ;)
>
> I did not find the relevant manpages where these bits of information
> could be added; but otherwise the task is tested and complete as far
> as I am concerned.

The first thing I noticed is: Illumos has a POSIX shell but its not
used as such. Further comments are inline within the patch diff:

--- /usr/lib/brand/ipkg/clone.orig      2011-09-12 15:01:44.000000000 +0400
+++ /usr/lib/brand/ipkg/clone   2013-11-13 05:31:27.756698164 +0400
@@ -45,9 +45,16 @@
 ROOT="rpool/ROOT"

 # Other brand clone options are invalid for this brand.
-while getopts "R:z:" opt; do
+while getopts "R:s:z:" opt; do
        case $opt in
                R)      ZONEPATH="$OPTARG" ;;
+               s)      case "$OPTARG" in
+                       *@*)    SNAPNAME="`echo "$OPTARG" | sed 's/^[^@]*@//'`"

You can use sed 's/^[^@]*@//' <<<"$OPTARG" instead of the pipe.

On a 2nd thought the shells ${var/string/repl} operator could replace
this completely if you could explain to me what you wish to do here.

+                               echo "WARNING: Ignoring dataset, using
only snapshot name"

Shouldn't this go to stderr?

+                               ;;
+                       *)      SNAPNAME="$OPTARG" ;;
+                       esac
+                       ;;
                z)      ZONENAME="$OPTARG" ;;
                *)      fail_usage "";;
        esac
@@ -96,19 +103,21 @@
 /usr/sbin/zfs create -o mountpoint=legacy -o zoned=on $zpds/$zpname/ROOT

 # make snapshot
-SNAPNAME=${ZONENAME}_snap
-SNAPNUM=0
-while [ $SNAPNUM -lt 100 ]; do
-       /usr/sbin/zfs snapshot $ACTIVE_DS@$SNAPNAME
-        if [ $? = 0 ]; then
-                break
-       fi
-       SNAPNUM=`expr $SNAPNUM + 1`
-       SNAPNAME="${ZONENAME}_snap$SNAPNUM"
-done
+if [ x"$SNAPNAME" = x ]; then
+       SNAPNAME=${ZONENAME}_snap
+       SNAPNUM=0
+       while [ $SNAPNUM -lt 100 ]; do

for ((SNAPNUM=0; SNAPNUM < 100;SNAPNUM++)) would be good

+               /usr/sbin/zfs snapshot $ACTIVE_DS@$SNAPNAME
+               if [ $? = 0 ]; then

if (( $? == 0 )); then

+                       break
+               fi
+               SNAPNUM=`expr $SNAPNUM + 1`
+               SNAPNAME="${ZONENAME}_snap$SNAPNUM"
+       done

-if [ $SNAPNUM -ge 100 ]; then
-       fail_fatal "$f_zfs_create"
+       if [ $SNAPNUM -ge 100 ]; then
+               fail_fatal "$f_zfs_create"
+       fi
 fi


Ced
--
Cedric Blancher <cedric.blancher at gmail.com>
Institute Pasteur


-- 
Cedric Blancher <cedric.blancher at gmail.com>
Institute Pasteur