)]}'
{"diskimage_builder/lib/disk-image-create":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"df3329e1e8fa5c7a99b16d341577d80c013ff567","unresolved":false,"context_lines":[{"line_number":422,"context_line":"if [ -n \"$DIB_JOURNAL_SIZE\" ]; then"},{"line_number":423,"context_line":"    journal_size\u003d\"$DIB_JOURNAL_SIZE\""},{"line_number":424,"context_line":"else"},{"line_number":425,"context_line":"    journal_size\u003d128"},{"line_number":426,"context_line":"fi"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"if [ \"$DIB_ROOT_FSTYPE\" \u003d \"ext4\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9fdfeff1_3452d6df","line":425,"updated":"2019-01-27 19:07:46.000000000","message":"Why do we need to increase this?\nI know at least Octavia builds 2-3GB images, which means this is bloating the image another 96MB that is unnecessary.","commit_id":"0d1445345d54fecf7013d7d22a375cd6e56d6ca1"},{"author":{"_account_id":17799,"name":"Logan V","email":"logan2211@gmail.com","username":"Logan2211"},"change_message_id":"9ba5866739f0a5b0cd4ba45c445a5c9aba206636","unresolved":false,"context_lines":[{"line_number":422,"context_line":"if [ -n \"$DIB_JOURNAL_SIZE\" ]; then"},{"line_number":423,"context_line":"    journal_size\u003d\"$DIB_JOURNAL_SIZE\""},{"line_number":424,"context_line":"else"},{"line_number":425,"context_line":"    journal_size\u003d128"},{"line_number":426,"context_line":"fi"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"if [ \"$DIB_ROOT_FSTYPE\" \u003d \"ext4\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9fdfeff1_65aae071","line":425,"in_reply_to":"9fdfeff1_3452d6df","updated":"2019-01-28 16:23:20.000000000","message":"It doesn\u0027t have to change, but I think 128M is a more sane default for typical VM images. I\u0027m open to leaving it as-is in this patch, so we can at least get the tunable in, and then propose the default journal size change separately if it is too contentious.\n\nAlso, please bear in mind that the size of the image does not necessarily correlate to what the size of the filesystem will be after cloud-init growpart has run. My images are well under 2GB, but the typical use case sees them deployed to anywhere from 256GB SSDs to 2-4TB bare metal disks, so the size of the image really does not have any bearing on what journal size we should use. The intended deployment of the image and resulting filesystem size should be the ultimate deciding factor.\n\nAnd, of course, with a tunable being added your image sizes will not need to be impacted. You would continue building smaller journaled images to meet your needs.\n\nI understand the Octavia images are a different use case, so that\u0027s why I\u0027m proposing we make this value easily tunable. DIB has no way of knowing my images will ultimately call for a 512M or 1G journal, just as it cannot know your images should have a 32M or 64M journal.","commit_id":"0d1445345d54fecf7013d7d22a375cd6e56d6ca1"},{"author":{"_account_id":17799,"name":"Logan V","email":"logan2211@gmail.com","username":"Logan2211"},"change_message_id":"49a4f11a1e1908a839f5f0521a16c82b6eb20191","unresolved":false,"context_lines":[{"line_number":422,"context_line":"if [ -n \"$DIB_JOURNAL_SIZE\" ]; then"},{"line_number":423,"context_line":"    journal_size\u003d\"$DIB_JOURNAL_SIZE\""},{"line_number":424,"context_line":"else"},{"line_number":425,"context_line":"    journal_size\u003d128"},{"line_number":426,"context_line":"fi"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"if [ \"$DIB_ROOT_FSTYPE\" \u003d \"ext4\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9fdfeff1_e870f2a6","line":425,"in_reply_to":"9fdfeff1_65aae071","updated":"2019-02-25 15:53:52.000000000","message":"Any feedback on this? If Octavia is building 2-3GB images then you would probably want a 32MB journal according to the mkfs.ext4 defaults linked in the commit message. It seems like you might want to consider not using the default journal size that DIB guesses, regardless whether we leave it at 64MB or increase to 128MB.","commit_id":"0d1445345d54fecf7013d7d22a375cd6e56d6ca1"},{"author":{"_account_id":17799,"name":"Logan V","email":"logan2211@gmail.com","username":"Logan2211"},"change_message_id":"43c3605e8227cd12f05e3248d3a2ba33a60e9e5a","unresolved":false,"context_lines":[{"line_number":422,"context_line":"if [ -n \"$DIB_JOURNAL_SIZE\" ]; then"},{"line_number":423,"context_line":"    journal_size\u003d\"$DIB_JOURNAL_SIZE\""},{"line_number":424,"context_line":"else"},{"line_number":425,"context_line":"    journal_size\u003d128"},{"line_number":426,"context_line":"fi"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"if [ \"$DIB_ROOT_FSTYPE\" \u003d \"ext4\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9fdfeff1_e609f683","line":425,"in_reply_to":"9fdfeff1_e870f2a6","updated":"2019-02-26 05:04:13.000000000","message":"I removed the default journal size change.","commit_id":"0d1445345d54fecf7013d7d22a375cd6e56d6ca1"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"df3329e1e8fa5c7a99b16d341577d80c013ff567","unresolved":false,"context_lines":[{"line_number":427,"context_line":""},{"line_number":428,"context_line":"if [ \"$DIB_ROOT_FSTYPE\" \u003d \"ext4\" ] ; then"},{"line_number":429,"context_line":"  # Very conservative to handle images being resized a lot"},{"line_number":430,"context_line":"  # We set journal size to 64M so our journal is large enough when we"},{"line_number":431,"context_line":"  # perform an FS resize."},{"line_number":432,"context_line":"  MKFS_OPTS\u003d\"-i 4096 -J size\u003d$journal_size $MKFS_OPTS\""},{"line_number":433,"context_line":"  # Grow the image size to account for the journal, only if the user"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9fdfeff1_744cde41","line":430,"updated":"2019-01-27 19:07:46.000000000","message":"This comment is no longer valid.","commit_id":"0d1445345d54fecf7013d7d22a375cd6e56d6ca1"},{"author":{"_account_id":17799,"name":"Logan V","email":"logan2211@gmail.com","username":"Logan2211"},"change_message_id":"9ba5866739f0a5b0cd4ba45c445a5c9aba206636","unresolved":false,"context_lines":[{"line_number":427,"context_line":""},{"line_number":428,"context_line":"if [ \"$DIB_ROOT_FSTYPE\" \u003d \"ext4\" ] ; then"},{"line_number":429,"context_line":"  # Very conservative to handle images being resized a lot"},{"line_number":430,"context_line":"  # We set journal size to 64M so our journal is large enough when we"},{"line_number":431,"context_line":"  # perform an FS resize."},{"line_number":432,"context_line":"  MKFS_OPTS\u003d\"-i 4096 -J size\u003d$journal_size $MKFS_OPTS\""},{"line_number":433,"context_line":"  # Grow the image size to account for the journal, only if the user"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9fdfeff1_85a7c4a7","line":430,"in_reply_to":"9fdfeff1_744cde41","updated":"2019-01-28 16:23:20.000000000","message":"++ thanks, i\u0027ll update it in a later PS as needed depending on what direction we decide go with the journal size.","commit_id":"0d1445345d54fecf7013d7d22a375cd6e56d6ca1"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"42793b6f4ccd6e7888f0bf1e80486d70cc48d7db","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then"},{"line_number":455,"context_line":"    # For compatibily reasons in addition to the YAML configuration"},{"line_number":456,"context_line":"    # there is the need to handle the old environment variables."},{"line_number":457,"context_line":"    echo \"image-size: ${DIB_IMAGE_SIZE}KiB\" \u003e\u003e ${DIB_BLOCK_DEVICE_PARAMS_YAML}"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    if [ -n \"${MKFS_OPTS}\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"9fdfeff1_beb35ce9","line":456,"updated":"2019-03-05 06:55:12.000000000","message":"So I don\u0027t mean to be annoying, but I will be.  The block-device stuff was meant to deprecate these global variables so I\u0027m wary to keep adding to it.\n\nI most certainly understand why this is the easiest way :)\n\nHowever, we have a non-trivial amount of users that do fancy things with their partitions; multiple partitions including LVM setups with split logs and stuff like that.  This global variable is only useful for one use-case of a big single root partition and adds further confusion to the disk-layout options.\n\nWhat is actually happening is that [1] is being used to create the root disk.  We \"merge\" these legacy variables into that configuration as a backwards compatibility thing.  Right now you could set DIB_BLOCK_DEVICE_CONFIG\u003d to a YAML file with any options to mkfs that you want, including the changes required here, but also having different journal sizes for different partitions, etc.\n\nI understand you probably don\u0027t want to create your own partition file, which is why you proposed this\n\nOne compromise here might be if we expand variables where we copy the configuration file in [2]?  So instead of copying the file, we treat the input config file as some sort of a heredoc and any variables like ${DIB_BLOCK_DEVICE_MBR_ROOT_JOURNAL_SIZE} type variables could be expanded as the final output is written? \n\nI see this as being better because this means that block-device elements can define their own variables that make sense for their particular layout -- rather than a global flag that isn\u0027t actually global or useful for many users depending on their disk layout choices.\n\nI\u0027m open to other suggestions too, but I hope that makes sense \n\n[1] https://git.openstack.org/cgit/openstack/diskimage-builder/tree/diskimage_builder/elements/block-device-mbr/block-device-default.yaml\n[2] https://git.openstack.org/cgit/openstack/diskimage-builder/tree/diskimage_builder/lib/common-functions#n493","commit_id":"cd471f8667618e62fca2860884d9b7e0428078bb"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"d482f091e7a6655d1eedbcd8e591f761b98c93c7","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then"},{"line_number":455,"context_line":"    # For compatibily reasons in addition to the YAML configuration"},{"line_number":456,"context_line":"    # there is the need to handle the old environment variables."},{"line_number":457,"context_line":"    echo \"image-size: ${DIB_IMAGE_SIZE}KiB\" \u003e\u003e ${DIB_BLOCK_DEVICE_PARAMS_YAML}"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    if [ -n \"${MKFS_OPTS}\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"5fc1f717_2a3bc638","line":456,"in_reply_to":"5fc1f717_b9907a97","updated":"2019-03-06 04:45:13.000000000","message":"Hrm, yes I see your point here.  I\u0027m going to think about it, but am going to try looping in Yolanda for expertise on alternative disk layouts","commit_id":"cd471f8667618e62fca2860884d9b7e0428078bb"},{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"8ac72cb01a5ff57b679e7187fa7c0e24cf6f35c9","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then"},{"line_number":455,"context_line":"    # For compatibily reasons in addition to the YAML configuration"},{"line_number":456,"context_line":"    # there is the need to handle the old environment variables."},{"line_number":457,"context_line":"    echo \"image-size: ${DIB_IMAGE_SIZE}KiB\" \u003e\u003e ${DIB_BLOCK_DEVICE_PARAMS_YAML}"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    if [ -n \"${MKFS_OPTS}\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"5fc1f717_ded62dc4","line":456,"in_reply_to":"5fc1f717_b9907a97","updated":"2019-03-11 08:09:22.000000000","message":"I agree with Ian on that, better do it via block device definition. The image size would need to be increased considering the size of the journal partition as well. But once that is computed properly, it should work","commit_id":"cd471f8667618e62fca2860884d9b7e0428078bb"},{"author":{"_account_id":17799,"name":"Logan V","email":"logan2211@gmail.com","username":"Logan2211"},"change_message_id":"5b697ee5a634595f2506f4b4ed540b2d735cb8b3","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then"},{"line_number":455,"context_line":"    # For compatibily reasons in addition to the YAML configuration"},{"line_number":456,"context_line":"    # there is the need to handle the old environment variables."},{"line_number":457,"context_line":"    echo \"image-size: ${DIB_IMAGE_SIZE}KiB\" \u003e\u003e ${DIB_BLOCK_DEVICE_PARAMS_YAML}"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    if [ -n \"${MKFS_OPTS}\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"5fc1f717_51ce5d82","line":456,"in_reply_to":"5fc1f717_ded62dc4","updated":"2019-03-14 14:47:56.000000000","message":"I don\u0027t understand what you\u0027re proposing. Currently there is no way that I know of to access the computed image size in the block device definition, and so there is no way to \"increase\" it.","commit_id":"cd471f8667618e62fca2860884d9b7e0428078bb"},{"author":{"_account_id":17799,"name":"Logan V","email":"logan2211@gmail.com","username":"Logan2211"},"change_message_id":"1b5f6d1191d2ba84d494f9053cb2778e076aaec6","unresolved":false,"context_lines":[{"line_number":453,"context_line":""},{"line_number":454,"context_line":"if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then"},{"line_number":455,"context_line":"    # For compatibily reasons in addition to the YAML configuration"},{"line_number":456,"context_line":"    # there is the need to handle the old environment variables."},{"line_number":457,"context_line":"    echo \"image-size: ${DIB_IMAGE_SIZE}KiB\" \u003e\u003e ${DIB_BLOCK_DEVICE_PARAMS_YAML}"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    if [ -n \"${MKFS_OPTS}\" ] ; then"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"5fc1f717_b9907a97","line":456,"in_reply_to":"9fdfeff1_beb35ce9","updated":"2019-03-06 00:54:26.000000000","message":"I don\u0027t mind doing this via block device yaml if it would work. But it doesn\u0027t quite work.\n\nYes, I can override the journal size using the mkfs options in the block device yaml. However, the dynamic image size is then broken and does not adjust to accommodate a larger journal as it does with this change (L436).\n\nIs the legacy option the best way to do this, then? Or is there some way we can feed it through the block device yaml and maintain the image sizing correctly?","commit_id":"cd471f8667618e62fca2860884d9b7e0428078bb"}]}