)]}'
{"elements/lvm/README.rst":[{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"fccac61f13a4c1b2b012a248ea1faa5438750c0d","unresolved":false,"context_lines":[{"line_number":11,"context_line":"---------------------"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"DIB_LVM_VG_NAME"},{"line_number":14,"context_line":"  :Required: Yes"},{"line_number":15,"context_line":"  :Default: Name for the volume group."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"DIB_LVM_LV_NAME"}],"source_content_type":"text/x-rst","patch_set":23,"id":"da6ed579_8c5707ca","line":14,"updated":"2016-01-11 22:42:56.000000000","message":"Can we pick some sane deafults for this and LV_NAME? Maybe just main and root?","commit_id":"35be309ae5f9bf7c9ecc9989f55090e0893c5e3c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":13,"context_line":"DIB_LVM_SETTINGS"},{"line_number":14,"context_line":"  :Required: Yes"},{"line_number":15,"context_line":"  :Description: Multi-line string separated with commas. Each of the lines should have the format"},{"line_number":16,"context_line":"   \u003cpartition\u003e,\u003cvg_name\u003e,\u003clv_name\u003e,\u003cfilesystem\u003e,\u003coptions\u003e"},{"line_number":17,"context_line":"   \u003cpartition\u003e is the id of the previous sfdisk generated partition where to place the volume"},{"line_number":18,"context_line":"   \u003cvg_name\u003e is the name of the volume group"},{"line_number":19,"context_line":"   \u003clv_name\u003e is the name of the logical volume"}],"source_content_type":"text/x-rst","patch_set":34,"id":"dab17558_1c79eff6","line":16,"updated":"2016-05-09 21:28:02.000000000","message":"The concept does not work: IMHO the complete configuration need to modeled in another way. \nExample: one VG can contain some partitions.\nHow do you want to write this down?\np1,vg1,....\np2,vg1,....\nAnd how do you interpret the rest of the parameters?\n\nWhat should happen when somebody specifies?\np1,vg1,....\np1,vg2,....","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"dc97f82c1f2a8ea96916fcdcef6a82576d415703","unresolved":false,"context_lines":[{"line_number":13,"context_line":"DIB_LVM_SETTINGS"},{"line_number":14,"context_line":"  :Required: Yes"},{"line_number":15,"context_line":"  :Description: Multi-line string separated with commas. Each of the lines should have the format"},{"line_number":16,"context_line":"   \u003cpartition\u003e,\u003cvg_name\u003e,\u003clv_name\u003e,\u003cfilesystem\u003e,\u003coptions\u003e"},{"line_number":17,"context_line":"   \u003cpartition\u003e is the id of the previous sfdisk generated partition where to place the volume"},{"line_number":18,"context_line":"   \u003cvg_name\u003e is the name of the volume group"},{"line_number":19,"context_line":"   \u003clv_name\u003e is the name of the logical volume"}],"source_content_type":"text/x-rst","patch_set":34,"id":"bab6814e_b9729847","line":16,"in_reply_to":"bab6814e_637d216e","updated":"2016-05-19 08:32:08.000000000","message":"I allow to have multiple volumes, just repeating partition and volume group, and specify a different lvm. There is no place in my code where i reject it. I only restrict that one volume group needs to belong to a partition.\nI do not specify mount points for the volumes, that is not covered by my change, if you need to mount the volumes, that can be done on custom elements.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"1cdd88dc25075a9da6dd06466aed3fa5cabd693f","unresolved":false,"context_lines":[{"line_number":13,"context_line":"DIB_LVM_SETTINGS"},{"line_number":14,"context_line":"  :Required: Yes"},{"line_number":15,"context_line":"  :Description: Multi-line string separated with commas. Each of the lines should have the format"},{"line_number":16,"context_line":"   \u003cpartition\u003e,\u003cvg_name\u003e,\u003clv_name\u003e,\u003cfilesystem\u003e,\u003coptions\u003e"},{"line_number":17,"context_line":"   \u003cpartition\u003e is the id of the previous sfdisk generated partition where to place the volume"},{"line_number":18,"context_line":"   \u003cvg_name\u003e is the name of the volume group"},{"line_number":19,"context_line":"   \u003clv_name\u003e is the name of the logical volume"}],"source_content_type":"text/x-rst","patch_set":34,"id":"bab6814e_637d216e","line":16,"in_reply_to":"bab6814e_b6c49e40","updated":"2016-05-19 08:10:48.000000000","message":"The structure here is different from what you have in LVM:\nin LVM you assign one or more partitions to one volume group. Completely independent from this is the assignment of LV to VG.\nIf I understand you correctly to specify two lv within this schema can be configured as:\np2,sysvg,rootlv,ext4,-l100\np2,sysvg,tmplv,ext4,-l100\n\nIf you reject such kind of configuration, than you go to a solution where you can only have ONE partition per VG, and ONE LV per VG.\n\nEspecially the second restriction looks strange to me: this gives up one major benefit of using LVM.\n\nWhat I\u0027m also missing is the specification of the mount point for the LV.\nImagine to have the root fs as LV and the /var as another.\nHow do you specify which LV is mounted where?","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"5f892ebf5c5da64182fc4d78b6d93265be7c1b50","unresolved":false,"context_lines":[{"line_number":13,"context_line":"DIB_LVM_SETTINGS"},{"line_number":14,"context_line":"  :Required: Yes"},{"line_number":15,"context_line":"  :Description: Multi-line string separated with commas. Each of the lines should have the format"},{"line_number":16,"context_line":"   \u003cpartition\u003e,\u003cvg_name\u003e,\u003clv_name\u003e,\u003cfilesystem\u003e,\u003coptions\u003e"},{"line_number":17,"context_line":"   \u003cpartition\u003e is the id of the previous sfdisk generated partition where to place the volume"},{"line_number":18,"context_line":"   \u003cvg_name\u003e is the name of the volume group"},{"line_number":19,"context_line":"   \u003clv_name\u003e is the name of the logical volume"}],"source_content_type":"text/x-rst","patch_set":34,"id":"bab6814e_b6c49e40","line":16,"in_reply_to":"dab17558_1c79eff6","updated":"2016-05-18 10:35:43.000000000","message":"Why do you think this does not work? The idea is that you can create volumes under different partitions and volume groups. So with that schema you can just specify any partition, volume group and logical volumes you need. Do you see any problem with that?","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"1cdd88dc25075a9da6dd06466aed3fa5cabd693f","unresolved":false,"context_lines":[{"line_number":19,"context_line":"   \u003clv_name\u003e is the name of the logical volume"},{"line_number":20,"context_line":"   \u003cfilesystem\u003e is the filesystem type of the logical volume. If blank, no filesystem will be created"},{"line_number":21,"context_line":"   \u003coptions\u003e are the settings for the lvcreate format, and will vary depending on volume settings"},{"line_number":22,"context_line":"  :Default: p2,vg,lv,ext4,-l100 -n lv vg"}],"source_content_type":"text/x-rst","patch_set":34,"id":"bab6814e_038c1534","line":22,"updated":"2016-05-19 08:10:48.000000000","message":"One additional question:\nWhy is the default using p2? Why not p1? It sounds very uncommon to me to have a mixed setup using partitions AND LV.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"dc97f82c1f2a8ea96916fcdcef6a82576d415703","unresolved":false,"context_lines":[{"line_number":19,"context_line":"   \u003clv_name\u003e is the name of the logical volume"},{"line_number":20,"context_line":"   \u003cfilesystem\u003e is the filesystem type of the logical volume. If blank, no filesystem will be created"},{"line_number":21,"context_line":"   \u003coptions\u003e are the settings for the lvcreate format, and will vary depending on volume settings"},{"line_number":22,"context_line":"  :Default: p2,vg,lv,ext4,-l100 -n lv vg"}],"source_content_type":"text/x-rst","patch_set":34,"id":"bab6814e_19b764d9","line":22,"in_reply_to":"bab6814e_038c1534","updated":"2016-05-19 08:32:08.000000000","message":"So the idea for that, is that we partition the disk at least with two partitions. First one only for boot, and starting on second one, for extra volumes.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"}],"elements/lvm/block-device.d/10-lvm":[{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"a7ceb989e342fd448ec0603382da52d3c3cd5f6c","unresolved":false,"context_lines":[{"line_number":97,"context_line":"sudo pvremove --force \"${IMAGE_BLOCK_DEVICE}p2\""},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"# copy content again"},{"line_number":100,"context_line":"sudo dd if\u003d$TARGET_FILE of\u003d$IMAGE_BLOCK_DEVICE"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"# block device will be on first partition"},{"line_number":103,"context_line":"echo \"IMAGE_BLOCK_DEVICE\u003d${IMAGE_BLOCK_DEVICE}p1\""}],"source_content_type":"application/x-shellscript","patch_set":11,"id":"5a710552_f6929f9e","line":100,"updated":"2015-12-16 14:15:31.000000000","message":"Since it\u0027s not mounted anymore, you should be able to replace this with mv $TARGET_FILE $IMAGE_BLOCK_DEVICE","commit_id":"42dd75c24a717a39b6d0dcad80289c734eb75e06"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"d6ffd29b8b0ef2480391516393aa5e3bc3b6fe8f","unresolved":false,"context_lines":[{"line_number":24,"context_line":"source $_LIB/die"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"# sanity check validations"},{"line_number":27,"context_line":"[ -n \"$IMAGE_BLOCK_DEVICE\" ] || die \"Image block device not set\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"if [[ \"$ARCH\" \u003d~ \"ppc\" ]] ; then"},{"line_number":30,"context_line":"    die \"ppc architecture is not supported by lvm\""}],"source_content_type":"application/x-shellscript","patch_set":14,"id":"3a7e1126_7d889cc5","line":27,"updated":"2015-12-20 02:51:13.000000000","message":"I think this and the other die\u0027s do not do what we want - they are going to error and then exit 0 which means dib will continue with the other element scripts. I think we actually want dib to exit since the image will not be valid without a partition table.","commit_id":"889242760d713a416a7f23ab000fa78275943bf6"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"a73a8c494aec21eb449afd2c63fc021daaa388a3","unresolved":false,"context_lines":[{"line_number":24,"context_line":"source $_LIB/die"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"# sanity check validations"},{"line_number":27,"context_line":"[ -n \"$IMAGE_BLOCK_DEVICE\" ] || die \"Image block device not set\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"if [[ \"$ARCH\" \u003d~ \"ppc\" ]] ; then"},{"line_number":30,"context_line":"    die \"ppc architecture is not supported by lvm\""}],"source_content_type":"application/x-shellscript","patch_set":14,"id":"3a7e1126_c0df6738","line":27,"in_reply_to":"3a7e1126_7d889cc5","updated":"2015-12-20 05:41:54.000000000","message":"I did some even more digging and these might work, but there is a dib bug which prevents them from doing so: https://review.openstack.org/#/c/259734","commit_id":"889242760d713a416a7f23ab000fa78275943bf6"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"8de10741ef5a8a243633d734cc55db71426db8c4","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    die \"ppc architecture is not supported by lvm\""},{"line_number":31,"context_line":"fi"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"[ -n \"${IMAGE_BLOCK_DEVICE}\" ] || die \"Error: Imageblock device not set\""},{"line_number":34,"context_line":"[ -n \"${DIB_LVM_VG_NAME}\" ] || die \u0027Error: $DIB_LVM_VG_NAME not set\u0027"},{"line_number":35,"context_line":"[ -n \"${DIB_LVM_LV_NAME}\" ] || die \u0027Error: $DIB_LVM_VG_NAME not set\u0027"},{"line_number":36,"context_line":""}],"source_content_type":"application/x-shellscript","patch_set":14,"id":"3a7e1126_ddbd1069","line":33,"updated":"2015-12-20 02:52:52.000000000","message":"Is there a reason we are repeating this? I think it should just go away.","commit_id":"889242760d713a416a7f23ab000fa78275943bf6"}],"elements/lvm/block-device.d/50-lvm":[{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"7909c6820794a1af2299e7e8fc1d6ebfcb4e8794","unresolved":false,"context_lines":[{"line_number":42,"context_line":"fi"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"# create primary and logical partitions"},{"line_number":45,"context_line":"sudo sfdisk --force $IMAGE_BLOCK_DEVICE \u003c\u003c EOF"},{"line_number":46,"context_line":"0,150,L *"},{"line_number":47,"context_line":",,L"},{"line_number":48,"context_line":"0 0;"}],"source_content_type":"application/x-shellscript","patch_set":10,"id":"5a710552_fa12b508","line":45,"updated":"2015-12-15 18:40:04.000000000","message":"Could we get some explanation in comments of what this sfdisk block is intending to do?\n\n\"Create a 150 cyl partition and a partition expanded to max size.\"","commit_id":"7556c6ebab9e7e1f776392220e10b69e899327dd"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"7909c6820794a1af2299e7e8fc1d6ebfcb4e8794","unresolved":false,"context_lines":[{"line_number":94,"context_line":"sudo dd if\u003d$IMAGE_BLOCK_DEVICE of\u003d$TARGET_FILE"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"sudo lvremove --force $DIB_LVM_LV_NAME"},{"line_number":97,"context_line":"sudo vgremove --force $DIB_LVM_VG_NAME "},{"line_number":98,"context_line":"sudo pvremove --force \"${IMAGE_BLOCK_DEVICE}p2\""},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"# copy content again"}],"source_content_type":"application/x-shellscript","patch_set":10,"id":"5a710552_daa57904","line":97,"updated":"2015-12-15 18:40:04.000000000","message":"space","commit_id":"7556c6ebab9e7e1f776392220e10b69e899327dd"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"f8287956e616fc2411092939a65a81a77659d29f","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# need to copy contents to temporary file, to be"},{"line_number":58,"context_line":"# able to perform cleanup"},{"line_number":59,"context_line":"TARGET_FILE\u003d$(mktemp)"},{"line_number":60,"context_line":"sudo dd if\u003d$LOGICAL_PARTITION of\u003d$TARGET_FILE"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"sudo lvremove --force $DIB_LVM_LV_NAME || true"}],"source_content_type":"application/x-shellscript","patch_set":22,"id":"1a7b0d38_2d76043f","line":59,"updated":"2015-12-22 18:39:47.000000000","message":"Since this runs on the host, we don\u0027t have the luxury of a full $TMPDIR cleanout at the end. So we need to rm this after the dd.","commit_id":"267faf47cac8ce5564e4889ee2956ca382a1137d"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"fccac61f13a4c1b2b012a248ea1faa5438750c0d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"[ -n \"${DIB_LVM_LV_NAME}\" ] || die \u0027Error: $DIB_LVM_VG_NAME not set\u0027"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"if [ -z \"${DIB_LVM_LOGICAL_EXTENTS}\" ] \u0026\u0026 [ -z \"${DIB_LVM_LOGICAL_VOLUME_SIZE}\" ]; then"},{"line_number":33,"context_line":"    die \u0027Error: either $DIB_LVM_LOGICAL_EXTENTS or $DIB_LVM_LOGICAL_VOLUME_SIZE need to be set\u0027"},{"line_number":34,"context_line":"fi"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"LOGICAL_PARTITION\u003d\"${IMAGE_BLOCK_DEVICE}${DIB_LVM_PARTITION}\""}],"source_content_type":"application/x-shellscript","patch_set":23,"id":"da6ed579_cc497f90","line":33,"updated":"2016-01-11 22:42:56.000000000","message":"This does not actually exit with an exit 1 - it only prints the message and then exit 0\u0027s which means the build will continue. Needs to be false || die ... or echo \"message\" ; exit 1\n\nCan we also make this work by default? I would suggest if nothing is set then use 100% of the available space.","commit_id":"35be309ae5f9bf7c9ecc9989f55090e0893c5e3c"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"82228daf341242c91fed884c023bae55f15724bc","unresolved":false,"context_lines":[{"line_number":62,"context_line":"sudo lvremove --force /dev/${DIB_LVM_VG_NAME}/${DIB_LVM_LV_NAME}"},{"line_number":63,"context_line":"sudo vgremove --force ${DIB_LVM_VG_NAME}"},{"line_number":64,"context_line":"sudo pvremove --yes ${LOGICAL_PARTITION}"},{"line_number":65,"context_line":"sudo udevadm setttle"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"# move to final target"},{"line_number":68,"context_line":"sudo dd if\u003d${TARGET_FILE} of\u003d${IMAGE_BLOCK_DEVICE}"}],"source_content_type":"application/x-shellscript","patch_set":23,"id":"da6ed579_efa555b5","line":65,"updated":"2016-01-11 22:56:36.000000000","message":"s/setttle/settle","commit_id":"35be309ae5f9bf7c9ecc9989f55090e0893c5e3c"},{"author":{"_account_id":1726,"name":"Ghe Rivero","email":"ghe.rivero@gmail.com","username":"ghe.rivero"},"change_message_id":"38b8acdb2d1913bd4de39f241dc619f6a2efa10f","unresolved":false,"context_lines":[{"line_number":62,"context_line":"sudo lvremove --force /dev/${DIB_LVM_VG_NAME}/${DIB_LVM_LV_NAME}"},{"line_number":63,"context_line":"sudo vgremove --force ${DIB_LVM_VG_NAME}"},{"line_number":64,"context_line":"sudo pvremove --yes ${LOGICAL_PARTITION}"},{"line_number":65,"context_line":"sudo udevadm setttle"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"# move to final target"},{"line_number":68,"context_line":"sudo dd if\u003d${TARGET_FILE} of\u003d${IMAGE_BLOCK_DEVICE}"}],"source_content_type":"application/x-shellscript","patch_set":24,"id":"bae84128_385ecf76","line":65,"updated":"2016-02-24 23:06:01.000000000","message":"s/setttle/settle/","commit_id":"da162c88a2ef4a31e8ba95e184a1d5174304c768"},{"author":{"_account_id":1726,"name":"Ghe Rivero","email":"ghe.rivero@gmail.com","username":"ghe.rivero"},"change_message_id":"5d47e4dfdd2abeeef481446e95a6ed84985096ba","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"source $_LIB/die"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"# sanity check validations"},{"line_number":27,"context_line":"[ -n \"$IMAGE_BLOCK_DEVICE\" ] || die \"Image block device not set\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[ -n \"${DIB_LVM_VG_NAME}\" ] || die \u0027Error: $DIB_LVM_VG_NAME not set\u0027"}],"source_content_type":"application/x-shellscript","patch_set":26,"id":"bae84128_a1155ce6","line":26,"updated":"2016-02-25 16:16:52.000000000","message":"For this to work, it needs to pass the option -F to mkfs, via command line (---mkfs-opts) o env. variable (MKFS_OPTS)\n\nWe should check for it properly defined here and fail with a message if not present","commit_id":"91a4d52b6f16baca161d64f7280000c5591bc908"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"cf67e312d8c1fafbb931f886bdc784a9a7fb29b8","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Copyright 2015 Hewlett-Packard Development Company, L.P."},{"line_number":4,"context_line":"# All Rights Reserved."}],"source_content_type":"application/octet-stream","patch_set":32,"id":"3afc51ec_ad3a7434","line":1,"updated":"2016-03-13 20:41:47.000000000","message":"This needs to be #!/usr/local/bin/dib-python to handle python2/3 compat, and the element needs to depend on dib-python.","commit_id":"55186953e55eb8d091256489682e74ae1562da2c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                  \"\u003cpv_name\u003e,\u003cvg_name\u003e,\u003clv_name\u003e,\u003cpartition_type\u003e,\""},{"line_number":46,"context_line":"                  \"\u003coptions\u003e\")"},{"line_number":47,"context_line":"            sys.exit(1)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    # iterate over all settings"},{"line_number":50,"context_line":"    processed_pvs \u003d []"},{"line_number":51,"context_line":"    lvm_settings_set \u003d lvm_settings.strip().split(\u0027\\n\u0027)"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_fc4a2bab","line":48,"updated":"2016-05-09 21:28:02.000000000","message":"IMHO some checks are missing here like: is one partition only used once, has the same partition always the same vg specified, ....","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    lvm_settings_set \u003d lvm_settings.strip().split(\u0027\\n\u0027)"},{"line_number":52,"context_line":"    for lvm_setting in lvm_settings_set:"},{"line_number":53,"context_line":"        setting_set \u003d lvm_setting.strip().split(\u0027,\u0027)"},{"line_number":54,"context_line":"        pv_name \u003d setting_set[0]"},{"line_number":55,"context_line":"        vg_name \u003d setting_set[1]"},{"line_number":56,"context_line":"        lv_name \u003d setting_set[2]"},{"line_number":57,"context_line":"        filesystem \u003d setting_set[3]"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_f954fd21","line":54,"updated":"2016-05-09 21:28:02.000000000","message":"The name differs from the description in the readme: there you describe the first parameter as partition here as pv_name. (IMHO there is nothing like a name for a physical volume.)","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        options \u003d setting_set[4]"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        # start with physical volume creation"},{"line_number":61,"context_line":"        logical_partition \u003d \"%s%s\" % (image_block_device, pv_name)"},{"line_number":62,"context_line":"        if logical_partition not in processed_pvs:"},{"line_number":63,"context_line":"            subprocess.call([\u0027sudo\u0027, \u0027pvcreate\u0027, logical_partition])"},{"line_number":64,"context_line":"            processed_pvs.append(logical_partition)"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_dc148745","line":61,"updated":"2016-05-09 21:28:02.000000000","message":"AFAIK the IMAGE_BLOCK_DEVICE already contains the partition - so you add it here twice.\nRef. e.g. vm/block-image.d:\n echo \"IMAGE_BLOCK_DEVICE\u003d${IMAGE_BLOCK_DEVICE}p1\"","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"5f892ebf5c5da64182fc4d78b6d93265be7c1b50","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        options \u003d setting_set[4]"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        # start with physical volume creation"},{"line_number":61,"context_line":"        logical_partition \u003d \"%s%s\" % (image_block_device, pv_name)"},{"line_number":62,"context_line":"        if logical_partition not in processed_pvs:"},{"line_number":63,"context_line":"            subprocess.call([\u0027sudo\u0027, \u0027pvcreate\u0027, logical_partition])"},{"line_number":64,"context_line":"            processed_pvs.append(logical_partition)"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"bab6814e_b6581e58","line":61,"in_reply_to":"dab17558_dc148745","updated":"2016-05-18 10:35:43.000000000","message":"At the point this is called, IMAGE_BLOCK_DEVICE does not contain the partition.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        # start with physical volume creation"},{"line_number":61,"context_line":"        logical_partition \u003d \"%s%s\" % (image_block_device, pv_name)"},{"line_number":62,"context_line":"        if logical_partition not in processed_pvs:"},{"line_number":63,"context_line":"            subprocess.call([\u0027sudo\u0027, \u0027pvcreate\u0027, logical_partition])"},{"line_number":64,"context_line":"            processed_pvs.append(logical_partition)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # create volume group inside the physical volume"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_b9f555cc","line":63,"updated":"2016-05-09 21:28:02.000000000","message":"You need to check for the return code.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            processed_pvs.append(logical_partition)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # create volume group inside the physical volume"},{"line_number":67,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027vgcreate\u0027, vg_name, logical_partition])"},{"line_number":68,"context_line":"        subprocess.call(\u0027sudo lvcreate {options}\u0027.format("},{"line_number":69,"context_line":"            options\u003doptions, vg_name\u003dvg_name), shell\u003dTrue)"},{"line_number":70,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_fc686b89","line":67,"updated":"2016-05-09 21:28:02.000000000","message":"You need to check for the return code.\n\nAlso you need to add a similar mechanism as for PV: one volume group can be created only once. Additional physical volumes need to be added by pvextend.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # create volume group inside the physical volume"},{"line_number":67,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027vgcreate\u0027, vg_name, logical_partition])"},{"line_number":68,"context_line":"        subprocess.call(\u0027sudo lvcreate {options}\u0027.format("},{"line_number":69,"context_line":"            options\u003doptions, vg_name\u003dvg_name), shell\u003dTrue)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # generate filesystem"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_9c279f63","line":68,"updated":"2016-05-09 21:28:02.000000000","message":"You need to check for the return code.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            options\u003doptions, vg_name\u003dvg_name), shell\u003dTrue)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # generate filesystem"},{"line_number":72,"context_line":"        if os.path.exists(\u0027/dev/%s/%s\u0027 % (vg_name, lv_name)):"},{"line_number":73,"context_line":"            subprocess.call([\u0027sudo\u0027, \u0027mkfs.%s\u0027 % filesystem,"},{"line_number":74,"context_line":"                             \u0027/dev/%s/%s\u0027 % (vg_name, lv_name)])"},{"line_number":75,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027udevadm\u0027, \u0027settle\u0027])"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_5ca257af","line":72,"updated":"2016-05-09 21:28:02.000000000","message":"This test should IMHO the way, that if this does NOT exist, an error should be printed and sys.exit(1).\nCurrently you silently discard any problem / error.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        # generate filesystem"},{"line_number":72,"context_line":"        if os.path.exists(\u0027/dev/%s/%s\u0027 % (vg_name, lv_name)):"},{"line_number":73,"context_line":"            subprocess.call([\u0027sudo\u0027, \u0027mkfs.%s\u0027 % filesystem,"},{"line_number":74,"context_line":"                             \u0027/dev/%s/%s\u0027 % (vg_name, lv_name)])"},{"line_number":75,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027udevadm\u0027, \u0027settle\u0027])"},{"line_number":76,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_5cb7f7d7","line":73,"updated":"2016-05-09 21:28:02.000000000","message":"This is the wrong place to call mkfs.\nYou need to build first the complete VG (all VG need to be created and extended).","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027udevadm\u0027, \u0027settle\u0027])"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    # need to copy contents to temporary file, to be"},{"line_number":78,"context_line":"    # able to perform cleanup"},{"line_number":79,"context_line":"    target_file \u003d tempfile.NamedTemporaryFile(delete\u003dFalse)"},{"line_number":80,"context_line":"    target_file.close()"},{"line_number":81,"context_line":"    subprocess.call([\u0027dd\u0027, \u0027if\u003d%s\u0027 % image_block_device,"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_dc5f673f","line":78,"updated":"2016-05-09 21:28:02.000000000","message":"I do not understand why it should not be possible to cleanup things. Can you please explain the problem here in more detail?","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    # able to perform cleanup"},{"line_number":79,"context_line":"    target_file \u003d tempfile.NamedTemporaryFile(delete\u003dFalse)"},{"line_number":80,"context_line":"    target_file.close()"},{"line_number":81,"context_line":"    subprocess.call([\u0027dd\u0027, \u0027if\u003d%s\u0027 % image_block_device,"},{"line_number":82,"context_line":"                     \u0027of\u003d%s\u0027 % target_file.name])"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    # once copied, removed all volumes that have been created"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_1c83af99","line":81,"updated":"2016-05-09 21:28:02.000000000","message":"Copying everything a couple of times creates a lot of IO.\nAlso you should check first, if on the appropriate place is enough space.\n\nYou should check the result code.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"1cdd88dc25075a9da6dd06466aed3fa5cabd693f","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    # able to perform cleanup"},{"line_number":79,"context_line":"    target_file \u003d tempfile.NamedTemporaryFile(delete\u003dFalse)"},{"line_number":80,"context_line":"    target_file.close()"},{"line_number":81,"context_line":"    subprocess.call([\u0027dd\u0027, \u0027if\u003d%s\u0027 % image_block_device,"},{"line_number":82,"context_line":"                     \u0027of\u003d%s\u0027 % target_file.name])"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    # once copied, removed all volumes that have been created"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"bab6814e_1e9c6abf","line":81,"in_reply_to":"bab6814e_7141a039","updated":"2016-05-19 08:10:48.000000000","message":"Here you copy the underlying content of a mounted image!\nYou have *NO* guarantees that this will work. Before doing these things you need to unmount.\n\nOn the other hand there is really no need for doing this: you do not want to remove things here (which really removes the freshly created PV/VG/LV on disk!), but deactivate this.\nPlease have a look at my comment to e.g. the lvremove command further down.\n\nIMHO the way how it should be done is implemented in my patch during the cleanup phase.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"5f892ebf5c5da64182fc4d78b6d93265be7c1b50","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    # able to perform cleanup"},{"line_number":79,"context_line":"    target_file \u003d tempfile.NamedTemporaryFile(delete\u003dFalse)"},{"line_number":80,"context_line":"    target_file.close()"},{"line_number":81,"context_line":"    subprocess.call([\u0027dd\u0027, \u0027if\u003d%s\u0027 % image_block_device,"},{"line_number":82,"context_line":"                     \u0027of\u003d%s\u0027 % target_file.name])"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    # once copied, removed all volumes that have been created"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"bab6814e_7141a039","line":81,"in_reply_to":"dab17558_1c83af99","updated":"2016-05-18 10:35:43.000000000","message":"This is done, because environment needed to be cleaned after image is finished. If you don\u0027t dd , remove the volumes, and dd again, the host machine generating the image is dirty, with the volumes created on the host system. The way to avoid it was to do these steps.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"e7c31b3307ad59db014538c2a36601db91ac57cd","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        lv_name \u003d setting_set[2]"},{"line_number":90,"context_line":"        logical_partition \u003d \u0027%s%s\u0027 % (image_block_device, pv_name)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027lvremove\u0027, \u0027--force\u0027,"},{"line_number":93,"context_line":"                         \u0027/dev/%s/%s\u0027 % (vg_name, lv_name)])"},{"line_number":94,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027vgremove\u0027, \u0027--force\u0027, vg_name])"},{"line_number":95,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027pvremove\u0027, \u0027--yes\u0027, logical_partition])"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_ea2ecb8a","line":92,"updated":"2016-05-15 23:40:45.000000000","message":"lvremove is the wrong command here: you don\u0027t want to remove the LV.\nSame apply for vgremove and pvremove.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027lvremove\u0027, \u0027--force\u0027,"},{"line_number":93,"context_line":"                         \u0027/dev/%s/%s\u0027 % (vg_name, lv_name)])"},{"line_number":94,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027vgremove\u0027, \u0027--force\u0027, vg_name])"},{"line_number":95,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027pvremove\u0027, \u0027--yes\u0027, logical_partition])"},{"line_number":96,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027udevadm\u0027, \u0027settle\u0027])"},{"line_number":97,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_7c3f5bd6","line":94,"updated":"2016-05-09 21:28:02.000000000","message":"You have first to remove all LV before removing the VG.\n\n... and of course check for the result :-)","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027lvremove\u0027, \u0027--force\u0027,"},{"line_number":93,"context_line":"                         \u0027/dev/%s/%s\u0027 % (vg_name, lv_name)])"},{"line_number":94,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027vgremove\u0027, \u0027--force\u0027, vg_name])"},{"line_number":95,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027pvremove\u0027, \u0027--yes\u0027, logical_partition])"},{"line_number":96,"context_line":"        subprocess.call([\u0027sudo\u0027, \u0027udevadm\u0027, \u0027settle\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    # move to final target"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_a70fec2c","line":95,"updated":"2016-05-09 21:28:02.000000000","message":"... same here for VG and PV.","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"318747787cf69630d33ffc1fc3e8ba6f470bf28f","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    # move to final target"},{"line_number":99,"context_line":"    subprocess.call([\u0027dd\u0027, \u0027if\u003d%s\u0027 % target_file.name,"},{"line_number":100,"context_line":"                     \u0027of\u003d%s\u0027 % image_block_device])"},{"line_number":101,"context_line":"    os.unlink(target_file.name)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":104,"context_line":"    main()"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"dab17558_67a984df","line":101,"updated":"2016-05-09 21:28:02.000000000","message":"This might create a resource leak if something nasty happens in between the creation and deletion.\nA typical way of handling this is \u0027with ... as ...\u0027","commit_id":"57a06ab0ca1c552695b54acd5fa79056ea308a9c"}],"elements/lvm/element-deps":[{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"48b6440612968d40f21d1035cf919fefc7f00d78","unresolved":false,"context_lines":[{"line_number":1,"context_line":"bootloader"}],"source_content_type":"application/octet-stream","patch_set":14,"id":"3a7e1126_2a916cf1","line":1,"updated":"2015-12-19 23:36:14.000000000","message":"I would like for this to not depend on bootloader. In the VM element we need to do that for backwards compat but it would be awesome if we could allow users to make lvm based images with or without a bootloader installed.\n\nThis has some tricky bits to it - we either would need to always make the boot partition here or we need to have some logic to detect whether or not we should create one by introspecting the selected elements or by having the bootloader element export a var we check. IMO having the bootloader element export DIB_LVM_MAKE_BOOTLOADER_PARTITION\u003d1 seems like a reasonable solution, but I am interested to see if other people agree...","commit_id":"889242760d713a416a7f23ab000fa78275943bf6"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"123bfbb55cc79272e3755289dd82f32a77c4a74c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"bootloader"}],"source_content_type":"application/octet-stream","patch_set":14,"id":"3a7e1126_dbcc38bd","line":1,"in_reply_to":"3a7e1126_2a916cf1","updated":"2015-12-20 07:58:26.000000000","message":"Why not just remove totally, and document properly? If user wants bootloader , it will be enough with adding the element prior to lvm?","commit_id":"889242760d713a416a7f23ab000fa78275943bf6"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"991863b15a7f8dcb18040b1a8e18ac91b85dacae","unresolved":false,"context_lines":[{"line_number":1,"context_line":"bootloader"}],"source_content_type":"application/octet-stream","patch_set":14,"id":"3a7e1126_f276f2d5","line":1,"in_reply_to":"3a7e1126_dbcc38bd","updated":"2015-12-20 17:36:46.000000000","message":"Yes, we should remove this from the lvm element.","commit_id":"889242760d713a416a7f23ab000fa78275943bf6"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"cf67e312d8c1fafbb931f886bdc784a9a7fb29b8","unresolved":false,"context_lines":[{"line_number":1,"context_line":"partitioning-sfdisk"}],"source_content_type":"application/octet-stream","patch_set":32,"id":"3afc51ec_ed3c7c31","line":1,"updated":"2016-03-13 20:41:47.000000000","message":"This patch does not depend on a parent which has the partitioning-sfdisk element (so checking it out gives an error of \"ERROR: Element \u0027partitioning-sfdisk\u0027 not found\"). A rebase on master or the patch which added that element should fix this.","commit_id":"55186953e55eb8d091256489682e74ae1562da2c"}]}
