Allon Mureinik has posted comments on this change.

Change subject: core, restapi: Introducing support for attaching disk snapshot
......................................................................


Patch Set 31:

(5 comments)

See comment in create_views.sql - possible bug.

....................................................
Commit Message
Line 14: was used, if the user wanted to see the disk content at some snapshot 
he
Line 15: had to preview that snapshot.
Line 16: After this change, a snapshot of a disk can be attached to another vm,
Line 17: regardless of the disk not being marked as shareable - when doing so,
Line 18: VDSM should create a temp snapshot allowing read/write access above 
the selected snapshot, the above should happend when hotplugging a disk/ 
running a vm. In case of hot unplug of the disk snapshot vdsm should delete the 
temp snapshot.
Really REALLY LOOOOOOOOOOOOOONG line.
Please break it up.
Line 19: 
Line 20: The following limitations currently applies on that ability:
Line 21: 1. The created temp snapshots is stored on the host local storage (the 
host that the vm is running on) and not on the shared storage (domains) 
therefore the vm can't be migrated.
Line 22: 2. A disk snapshot can be attached to a different VM than the one of


Line 17: regardless of the disk not being marked as shareable - when doing so,
Line 18: VDSM should create a temp snapshot allowing read/write access above 
the selected snapshot, the above should happend when hotplugging a disk/ 
running a vm. In case of hot unplug of the disk snapshot vdsm should delete the 
temp snapshot.
Line 19: 
Line 20: The following limitations currently applies on that ability:
Line 21: 1. The created temp snapshots is stored on the host local storage (the 
host that the vm is running on) and not on the shared storage (domains) 
therefore the vm can't be migrated.
here too.
Line 22: 2. A disk snapshot can be attached to a different VM than the one of
Line 23: which the snapshot (VM snapshot) was taken of.
Line 24: 
Line 25: Usage:


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
Line 268:                 devices,
Line 269:                 getSrcDeviceIdToTargetDeviceIdMapping(),
Line 270:                 getParameters().isSoundDeviceEnabled(),
Line 271:                 getParameters().isConsoleEnabled(),
Line 272:                 isVirtioScsiEnabled(), false);
false should be on its own line.
Line 273:     }
Line 274: 
Line 275:     @Override
Line 276:     protected VdcActionType getChildActionType() {


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
Line 308:     }
Line 309: 
Line 310:     /**
Line 311:      * A general method for run vm validations. used in runVmCommand 
and in VmPoolCommandBase
Line 312:      *
does this fly with checkstyle?
Line 313:      * @param vm
Line 314:      * @param messages
Line 315:      * @param vmDisks
Line 316:      * @param bootSequence


....................................................
File packaging/dbscripts/create_views.sql
Line 77:     base_disks.last_alignment_scan as last_alignment_scan
Line 78: FROM
Line 79: images
Line 80: left outer join disk_image_dynamic on images.image_guid = 
disk_image_dynamic.image_id
Line 81: LEFT OUTER JOIN base_disks ON images.image_group_id = 
base_disks.disk_id
This view will now return an entry for each VM a snapshot is attached to.
IIUC, this make break functionality that depends on the view, such as 
getAllSnapshotsForVmSnapshot
Line 82: LEFT OUTER JOIN vms_for_disk_view on vms_for_disk_view.device_id = 
images.image_group_id
Line 83: LEFT JOIN image_storage_domain_map ON 
image_storage_domain_map.image_id = images.image_guid
Line 84: LEFT OUTER JOIN storage_domain_static_view ON 
image_storage_domain_map.storage_domain_id = storage_domain_static_view.id
Line 85: LEFT OUTER JOIN snapshots ON images.vm_snapshot_id = 
snapshots.snapshot_id


-- 
To view, visit http://gerrit.ovirt.org/17679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I02579bf1a91cd294a5040acf432f1fdb87eb18c1
Gerrit-PatchSet: 31
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to