Add new API: createImage()

As it currently stands, images are created implicitly when a volume is created
using a new imageUUID.  Although this is practical for the current use cases, it
doesn't present a semetrical API.  Other vdsm objects have create/delete
methods.i

Furthermore, it preents challenges when trying to model VDSM in a REST API.
Let's say I want to create a new disk for a VM.  I would want to issue the
following sequence of commands:

POST -> /vdsm-api/storagedomains/<sdUUID>/images/create
<- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
GET -> /vdsm-api/tasks/<task-uuid>
<- HTTP/200 OK: /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>
POST -> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/create
<- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
GET -> /vdsm-api/tasks/<task-uuid>
<- HTTP/200 OK:
    
/vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/<volUUID>

The only way to support this pattern is to allow for the explicit creation of an
image with an empty volume chain.

Signed-off-by: Adam Litke <agl(a)us.ibm.com&gt;

diff --git a/vdsm/storage/spm.py b/vdsm/storage/spm.py
index 849b9d2..1222752 100644
--- a/vdsm/storage/spm.py
+++ b/vdsm/storage/spm.py
@@ -820,6 +820,23 @@ class SPM:
             repoPath = os.path.join(self.storage_repository, spUUID)
             image.Image(repoPath).multiMove(srcDomUUID, dstDomUUID, imgDict, vmUUID,
force)
 
+    def createImage(self, sdUUID, spUUID, imgUUID):
+        """
+        Create a new, empty image
+
+        :param sdUUID: The UUID of the storage domain that contains the images.
+        :type sdUUID: UUID
+        :param spUUID: The UUID of the storage pool that contains the images.
+        :type spUUID: UUID
+        :param imgUUID: The UUID of the image you want to delete.
+        :type imgUUID: UUID
+
+        """
+        hsm.HSM.getPool(spUUID) #Validates that the pool is connected. WHY?
+        hsm.HSM.validateSdUUID(sdUUID)
+        repoPath = os.path.join(self.storage_repository, spUUID)
+        image.Image(repoPath).create(sdUUID, imgUUID)
+        return dict(uuid=imgUUID)
 
     def deleteImage(self, sdUUID, spUUID, imgUUID, postZero, force):
         """
@@ -1470,6 +1487,18 @@ class SPM:
             imgUUID, volumes, misc.parseBool(postZero), misc.parseBool(force)
         )
 
+    def public_createImage(self, sdUUID, spUUID, imgUUID):
+        """
+        Create a new, empty image
+        """
+        argsStr = "sdUUID=%s, spUUID=%s, imgUUID=%s" % \
+                  (str(sdUUID), str(spUUID), str(imgUUID))
+        vars.task.setDefaultException(se.ImagePathError(argsStr))
+        hsm.HSM.getPool(spUUID) #Validates that the pool is connected. WHY?
+        hsm.HSM.validateSdUUID(sdUUID)
+        misc.validateUUID(imgUUID, 'imgUUID')
+        vars.task.getSharedLock(STORAGE, sdUUID)
+        self._schedule("createImage", self.createImage, sdUUID, spUUID,
imgUUID)
 
     def public_deleteImage(self, sdUUID, spUUID, imgUUID, postZero=False, force=False):
         """
-- 
Adam Litke <agl(a)us.ibm.com&gt;
IBM Linux Technology Center

On 09/21/2011 04:58 AM, Adam Litke wrote:
>  As it currently stands, images are created implicitly when a volume
> is created
>  using a new imageUUID.  Although this is practical for the current use cases, it
>  doesn't present a semetrical API.  Other vdsm objects have create/delete
>  methods.i
>
>  Furthermore, it preents challenges when trying to model VDSM in a REST API.
>  Let's say I want to create a new disk for a VM.  I would want to issue the
>  following sequence of commands:
>
>  POST ->  /vdsm-api/storagedomains/<sdUUID>/images/create
>  <- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
>  GET ->  /vdsm-api/tasks/<task-uuid>
>  <- HTTP/200 OK: /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>
>  POST -> 
> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/create
>  <- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
>  GET ->  /vdsm-api/tasks/<task-uuid>
>  <- HTTP/200 OK:
>       
> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/<volUUID>
>
>  The only way to support this pattern is to allow for the explicit creation of an
>  image with an empty volume chain.
>
>  Signed-off-by: Adam Litke&lt;agl(a)us.ibm.com&gt;
>
>  diff --git a/vdsm/storage/spm.py b/vdsm/storage/spm.py
>  index 849b9d2..1222752 100644
>  --- a/vdsm/storage/spm.py
>  +++ b/vdsm/storage/spm.py
>  @@ -820,6 +820,23 @@ class SPM:
>                repoPath = os.path.join(self.storage_repository, spUUID)
>                image.Image(repoPath).multiMove(srcDomUUID, dstDomUUID, imgDict, vmUUID,
> force)
>
>  +    def createImage(self, sdUUID, spUUID, imgUUID):
>  +        """
>  +        Create a new, empty image
>  +
>  +        :param sdUUID: The UUID of the storage domain that contains the images.
>  +        :type sdUUID: UUID
>  +        :param spUUID: The UUID of the storage pool that contains the images.
>  +        :type spUUID: UUID
>  +        :param imgUUID: The UUID of the image you want to delete. 
why delete?
Y.

>  +        :type imgUUID: UUID
>  +
>  +        """
>  +        hsm.HSM.getPool(spUUID) #Validates that the pool is connected. WHY?
>  +        hsm.HSM.validateSdUUID(sdUUID)
>  +        repoPath = os.path.join(self.storage_repository, spUUID)
>  +        image.Image(repoPath).create(sdUUID, imgUUID)
>  +        return dict(uuid=imgUUID)
>
>        def deleteImage(self, sdUUID, spUUID, imgUUID, postZero, force):
>            """
>  @@ -1470,6 +1487,18 @@ class SPM:
>                imgUUID, volumes, misc.parseBool(postZero), misc.parseBool(force)
>            )
>
>  +    def public_createImage(self, sdUUID, spUUID, imgUUID):
>  +        """
>  +        Create a new, empty image
>  +        """
>  +        argsStr = "sdUUID=%s, spUUID=%s, imgUUID=%s" % \
>  +                  (str(sdUUID), str(spUUID), str(imgUUID))
>  +        vars.task.setDefaultException(se.ImagePathError(argsStr))
>  +        hsm.HSM.getPool(spUUID) #Validates that the pool is connected. WHY?
>  +        hsm.HSM.validateSdUUID(sdUUID)
>  +        misc.validateUUID(imgUUID, 'imgUUID')
>  +        vars.task.getSharedLock(STORAGE, sdUUID)
>  +        self._schedule("createImage", self.createImage, sdUUID, spUUID,
> imgUUID)
>
>        def public_deleteImage(self, sdUUID, spUUID, imgUUID, postZero=False,
> force=False):
>            """

On Wed 21 Sep 2011 07:10:28 AM IDT, Yaniv Kaul wrote:
>  On 09/21/2011 04:58 AM, Adam Litke wrote:
> > As it currently stands, images are created implicitly when a volume is created
> > using a new imageUUID.  Although this is practical for the current use cases, it
> > doesn't present a semetrical API.  Other vdsm objects have create/delete
> > methods.i
> >
> > Furthermore, it preents challenges when trying to model VDSM in a REST API.
> > Let's say I want to create a new disk for a VM.  I would want to issue the
> > following sequence of commands:
> >
> > POST ->   /vdsm-api/storagedomains/<sdUUID>/images/create
> > <- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
> > GET ->   /vdsm-api/tasks/<task-uuid>
> > <- HTTP/200 OK: /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>
> > POST ->  
> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/create
> > <- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
> > GET ->   /vdsm-api/tasks/<task-uuid>
> > <- HTTP/200 OK:
> >       
> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/<volUUID>
> >
> > The only way to support this pattern is to allow for the explicit creation of an
> > image with an empty volume chain.
> >
> > Signed-off-by: Adam Litke&lt;agl(a)us.ibm.com&gt;
> >
> > diff --git a/vdsm/storage/spm.py b/vdsm/storage/spm.py
> > index 849b9d2..1222752 100644
> > --- a/vdsm/storage/spm.py
> > +++ b/vdsm/storage/spm.py
> > @@ -820,6 +820,23 @@ class SPM:
> >                repoPath = os.path.join(self.storage_repository, spUUID)
> >                image.Image(repoPath).multiMove(srcDomUUID, dstDomUUID, imgDict,
> vmUUID, force)
> >
> > +    def createImage(self, sdUUID, spUUID, imgUUID):
> > +        """
> > +        Create a new, empty image
> > +
> > +        :param sdUUID: The UUID of the storage domain that contains the images.
> > +        :type sdUUID: UUID
> > +        :param spUUID: The UUID of the storage pool that contains the images.
> > +        :type spUUID: UUID
> > +        :param imgUUID: The UUID of the image you want to delete.
>
>  why delete?
>  Y.
>
> > +        :type imgUUID: UUID
> > +
> > +        """
> > +        hsm.HSM.getPool(spUUID) #Validates that the pool is connected. WHY?
> > +        hsm.HSM.validateSdUUID(sdUUID)
> > +        repoPath = os.path.join(self.storage_repository, spUUID)
> > +        image.Image(repoPath).create(sdUUID, imgUUID)
> > +        return dict(uuid=imgUUID)
> >
> >        def deleteImage(self, sdUUID, spUUID, imgUUID, postZero, force):
> >            """
> > @@ -1470,6 +1487,18 @@ class SPM:
> >                imgUUID, volumes, misc.parseBool(postZero), misc.parseBool(force)
> >            )
> >
> > +    def public_createImage(self, sdUUID, spUUID, imgUUID):
> > +        """
> > +        Create a new, empty image
> > +        """
> > +        argsStr = "sdUUID=%s, spUUID=%s, imgUUID=%s" % \
> > +                  (str(sdUUID), str(spUUID), str(imgUUID))
> > +        vars.task.setDefaultException(se.ImagePathError(argsStr))
> > +        hsm.HSM.getPool(spUUID) #Validates that the pool is connected. WHY?
> > +        hsm.HSM.validateSdUUID(sdUUID)
> > +        misc.validateUUID(imgUUID, 'imgUUID')
> > +        vars.task.getSharedLock(STORAGE, sdUUID)
> > +        self._schedule("createImage", self.createImage, sdUUID, spUUID,
> imgUUID)
> >
> >        def public_deleteImage(self, sdUUID, spUUID, imgUUID, postZero=False,
> force=False):
> >            """
>
>  _______________________________________________
>  vdsm-devel mailing list
>  vdsm-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/vdsm-devel 
Tanks for the suggestion. The problem your pointing out is not the only 
problem with the current images API. We are hard at work at designing a 
new API.

Introducing a new API call at this point will just give us one more 
thing to support when we make the change.
We will send an RFC doc once we have a concrete view that we are happy 
with.

On Wed, Sep 21, 2011 at 07:10:28AM +0300, Yaniv Kaul wrote:
>  On 09/21/2011 04:58 AM, Adam Litke wrote:
>  >As it currently stands, images are created implicitly when a volume is created
>  >using a new imageUUID.  Although this is practical for the current use cases, it
>  >doesn't present a semetrical API.  Other vdsm objects have create/delete
>  >methods.i
>  >
>  >Furthermore, it preents challenges when trying to model VDSM in a REST API.
>  >Let's say I want to create a new disk for a VM.  I would want to issue the
>  >following sequence of commands:
>  >
>  >POST ->  /vdsm-api/storagedomains/<sdUUID>/images/create
>  ><- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
>  >GET ->  /vdsm-api/tasks/<task-uuid>
>  ><- HTTP/200 OK: /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>
>  >POST -> 
> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/create
>  ><- HTTP/202 Accepted: /vdsm-api/tasks/<task-uuid>
>  >GET ->  /vdsm-api/tasks/<task-uuid>
>  ><- HTTP/200 OK:
>  >     
> /vdsm-api/storagedomains/<sdUUID>/images/<imgUUID>/volumes/<volUUID>
>  >
>  >The only way to support this pattern is to allow for the explicit creation of an
>  >image with an empty volume chain.
>  >
>  >Signed-off-by: Adam Litke&lt;agl(a)us.ibm.com&gt;
>  >
>  >diff --git a/vdsm/storage/spm.py b/vdsm/storage/spm.py
>  >index 849b9d2..1222752 100644
>  >--- a/vdsm/storage/spm.py
>  >+++ b/vdsm/storage/spm.py
>  >@@ -820,6 +820,23 @@ class SPM:
>  >              repoPath = os.path.join(self.storage_repository, spUUID)
>  >              image.Image(repoPath).multiMove(srcDomUUID, dstDomUUID, imgDict,
> vmUUID, force)
>  >
>  >+    def createImage(self, sdUUID, spUUID, imgUUID):
>  >+        """
>  >+        Create a new, empty image
>  >+
>  >+        :param sdUUID: The UUID of the storage domain that contains the images.
>  >+        :type sdUUID: UUID
>  >+        :param spUUID: The UUID of the storage pool that contains the images.
>  >+        :type spUUID: UUID
>  >+        :param imgUUID: The UUID of the image you want to delete.
>  
>  why delete? 
Oops.  That was a result of a hasty copy/paste of arguments documentation from
deleteImage() :)  I will fix.

-- 
Adam Litke <agl(a)us.ibm.com&gt;
IBM Linux Technology Center

On Wed, Sep 21, 2011 at 01:41:11PM +0300, Saggi Mizrahi wrote:
>  Tanks for the suggestion. The problem your pointing out is not the
> only 
>  problem with the current images API. We are hard at work at designing a 
>  new API.
>  
>  Introducing a new API call at this point will just give us one more 
>  thing to support when we make the change.
>  We will send an RFC doc once we have a concrete view that we are happy 
>  with. 
Since this is now an open source project, is there any chance that these API
discussions could happen on the list where us new stakeholders can participate?

-- 
Adam Litke <agl(a)us.ibm.com&gt;
IBM Linux Technology Center