-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue #129 Workaround for virt-sparsify to work with immutable partition #130
Conversation
# TMPDIR must point at a directory with as much free space as the size of the image we want to sparsify | ||
# Read limitation section of `man virt-sparsify`. | ||
TMPDIR=$(pwd)/$destDir ${VIRT_SPARSIFY} -o lazy_refcounts=on $destDir/${CRC_VM_NAME}.qcow2 $destDir/${CRC_VM_NAME}_sparse.qcow2 | ||
rm -f $destDir/${CRC_VM_NAME}.qcow2 | ||
mv $destDir/${CRC_VM_NAME}_sparse.qcow2 $destDir/${CRC_VM_NAME}.qcow2 | ||
rm -fr $destDir/.guestfs-* | ||
|
||
# Set immutable attribute back for /dev/sda4 | ||
guestfish -a $destDir/${CRC_VM_NAME}.qcow2 -m /dev/sda4 set-e2attrs / i | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't that cause issues with 4.2 which does not set this bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cfergeau I didn't test that but it shouldn't if we just want to make that partition as immutable, Need to test this out and if this is causing issue we might just create another branch for 4.2 side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 4.2 the partition is not immutable, so I don't know how coreos is going to behave if we suddenly make it immutable. We could check whether the partition is immutable or not before making these changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cfergeau on 4.2 side there is no /dev/sda4
, so in that case it is failed and ignored I hope.
$ guestfish -a crc.qcow2 -m /dev/sda4 set-e2attrs / i
libguestfs: error: vfs_type: vfs_type_stub: /dev/sda4: No such file or directory
libguestfs: error: mount_options: mount_options_stub: /dev/sda4: No such file or directory
guestfish: ‘/dev/sda4’ could not be mounted.
guestfish: Did you mean to mount one of these filesystems?
guestfish: /dev/sda1 (unknown)
guestfish: /dev/sda2 (ext4)
guestfish: /dev/sda3 (xfs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 4.2 the partition is not immutable, so I don't know how coreos is going to behave if we suddenly make it immutable. We could check whether the partition is immutable or not before making these changes.
@cfergeau yes, that would be a better solution in terms of checks. Using the guestfish we can get if there is immutable bit set and then only perform removing that bit and adding it but as I said for the 4.2 side there is no /dev/sda4
.
$ guestfish -a crc.qcow2 -m /dev/sda4 get-e2attrs /
i
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For 4.3 images
guestfish <<_EOF_
add crc.qcow2
run
list-filesystems
_EOF_
/dev/sda1: ext4
/dev/sda2: vfat
/dev/sda3: unknown
/dev/sda4: xfs
For 4.2 images
$ guestfish <<_EOF_
> add crc.qcow2
> run
> list-filesystems
> _EOF_
/dev/sda1: unknown
/dev/sda2: ext4
/dev/sda3: xfs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh well, if no /dev/sda4 on 4.2, this indeed means my concern is not going to be an issue. We could leave it as it is and ignore the error.
Looks like we should double check we resize the correct partition on 4.3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we should double check we resize the correct partition on 4.3?
Yes now it should be /dev/sda4
which we need to resize /dev/sda3
is the boot partition so I think first we detect which partition is xfs
and then increase it size.
$ sudo fdisk -l
Disk /dev/vda: 31 GiB, 33285996544 bytes, 65011712 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: gpt
Disk identifier: 00000000-0000-4000-A000-000000000001
Device Start End Sectors Size Type
/dev/vda1 2048 788479 786432 384M Linux filesystem
/dev/vda2 788480 1048575 260096 127M EFI System
/dev/vda3 1048576 32505215 31456640 15G BIOS boot
/dev/vda4 32505216 65008990 32503775 15.5G Linux filesystem
In 4.2 the root partition mounted on `/dev/sda3` but for 4.3 it is mounted on `/dev/sda4` so to work properly with both the version, it is good to detect first and then use for `virt-resize`. - 4.2 image ``` $ virt-filesystems -a crc.qcow2 -l Name Type VFS Label Size Parent /dev/sda2 filesystem ext4 boot 1073741824 - /dev/sda3 filesystem xfs root 32208846848 - ``` - 4.3 image ``` $ virt-filesystems -a crc.qcow2 -l Name Type VFS Label Size Parent /dev/sda1 filesystem ext4 boot 402653184 - /dev/sda2 filesystem vfat EFI-SYSTEM 133169152 - /dev/sda4 filesystem xfs root 16641932800 - ```
fc71644
to
b5499ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still one concern with the immutable bit and growing the partition, but it's most likely a lack of understanding of how things are setup once the rhcos image is running.
if ! which ${VIRT_FILESYSTEMS}; then | ||
sudo yum -y install /usr/bin/virt-filesystems | ||
fi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the same package as virt-resize, so most of the time this will be skipped.
@@ -50,9 +50,12 @@ function create_qemu_image { | |||
${QEMU_IMG} rebase -b ${VM_PREFIX}-base $destDir/${VM_PREFIX}-master-0 | |||
${QEMU_IMG} commit $destDir/${VM_PREFIX}-master-0 | |||
|
|||
# Check which partition is labeled as `root` | |||
partition=$(${VIRT_FILESYSTEMS} -a $destDir/${VM_PREFIX}-base -l | grep root | cut -f1 -d' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awk
could be used instead of cut
, both are fine with me.
# Resize the image from the default 1+15GB to 1+30GB | ||
${QEMU_IMG} create -o lazy_refcounts=on -f qcow2 $destDir/${CRC_VM_NAME}.qcow2 31G | ||
${VIRT_RESIZE} --expand /dev/sda3 $destDir/${VM_PREFIX}-base $destDir/${CRC_VM_NAME}.qcow2 | ||
${VIRT_RESIZE} --expand $partition $destDir/${VM_PREFIX}-base $destDir/${CRC_VM_NAME}.qcow2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other commit fixes an issue where /dev/sda4 is immutable with openshift 4.3. Can we write to that partition after the cluster is setup? Otherwise, it's not really useful to grow it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misunderstanding on my side of how the 'i' attribute works, it's only set on the 'root' directory of the /dev/sda4 partition, so we cannot write to that dir. However, it's not recursive, so this does not impact any of its subdirectories which do not have that bit set. So this is not going to be an issue.
createdisk.sh
Outdated
@@ -61,13 +61,19 @@ function create_qemu_image { | |||
exit 1 | |||
fi | |||
|
|||
# Remove immutable attribute for /dev/sda4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe replace /dev/sda4 with "OpenShift 4.3+"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I understand better how the 'i' attribute works, only one very minor comment left on one comments, so ACK.
With OpenShift-4.3 the rh-coreos image failed during sparsify because of the immutable attribute set for /dev/sda4 partition. Need to use guestfish to remove it and run the virt-sparsify and then apply back. - https://bugzilla.redhat.com/show_bug.cgi?id=1771257
b5499ca
to
b178df3
Compare
With OpenShift-4.3 the rh-coreos image failed during sparsify because of
the immutable attribute set for /dev/sda4 partition. Need to use guestfish
to remove it and run the virt-sparsify and then apply back.