-
Notifications
You must be signed in to change notification settings - Fork 86
Updates for libvirt image removal and use directory #1722
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
base: master
Are you sure you want to change the base?
Changes from all commits
8b495f8
6663975
12da995
81dd45a
c1cf813
ea49630
f2822c7
28ec848
de53cf4
3ffebdf
a9d8167
7b03382
fca221a
d5d3bad
79e2e53
7256297
00ed5d2
d13e6b2
a41622b
092af90
6d7c50c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1711,7 +1711,11 @@ Stop VMs from Autostarting\Starting when VM Manager starts or open is run from t | |||||||||||||
| :end | ||||||||||||||
|
|
||||||||||||||
| :vms_libvirt_volume_help: | ||||||||||||||
| This is the libvirt volume. | ||||||||||||||
| This is the libvirt volume/directory. | ||||||||||||||
| :end | ||||||||||||||
|
|
||||||||||||||
| :vms_libvirt_secondary_volume_help: | ||||||||||||||
| This is a location for storing previous versions of xml and nvram at change. | ||||||||||||||
| :end | ||||||||||||||
|
|
||||||||||||||
| :vms_libvirt_vdisk_size_help: | ||||||||||||||
|
|
@@ -1720,7 +1724,11 @@ To resize an existing image file, specify the new size here. Next time the Libvi | |||||||||||||
| :end | ||||||||||||||
|
|
||||||||||||||
| :vms_libvirt_location_help: | ||||||||||||||
| You must specify an image file for Libvirt. The system will automatically create this file when the Libvirt service is first started. | ||||||||||||||
| You must specify an image file/directory for Libvirt. The system will automatically create this file/directory when the Libvirt service is first started. | ||||||||||||||
| :end | ||||||||||||||
|
|
||||||||||||||
| :vms_libvirt_secondary_location_help: | ||||||||||||||
| This is a directory for storing previous versions of xml and nvram at change. Does not need to be specified. | ||||||||||||||
| :end | ||||||||||||||
|
Comment on lines
+1730
to
1732
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix grammar: "at change" is awkward. The phrase "at change" should be replaced with clearer alternatives. This issue was previously flagged and remains unresolved. Additionally, "XML" and "NVRAM" should be capitalized for consistency with the rest of the documentation. 📝 Suggested fix :vms_libvirt_secondary_location_help:
- This is a directory for storing previous versions of xml and nvram at change. Does not need to be specified.
+ This is a directory for storing previous versions of XML and NVRAM when changes are made. Does not need to be specified.Or alternatively: - This is a directory for storing previous versions of xml and nvram at change. Does not need to be specified.
+ This is a directory for storing previous versions of XML and NVRAM on change; it does not need to be specified.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| :vms_libvirt_storage_help: | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -123,7 +123,7 @@ $libvirt_log = file_exists("/var/log/libvirt/libvirtd.log"); | |||||
| <input type="hidden" name="#file" value="<?=htmlspecialchars($domain_cfgfile)?>"> | ||||||
| <input type="hidden" name="#command" value="/plugins/dynamix/scripts/emcmd"> | ||||||
| <input type="hidden" name="#arg[1]" value="cmdStatus=Apply"> | ||||||
|
|
||||||
| <input type="hidden" id="OLD_IMAGE_FILE" name="OLD_IMAGE_FILE" value="<?=htmlspecialchars($domain_cfg['IMAGE_FILE']);?>"> | ||||||
| _(Enable VMs)_: | ||||||
| : <select id="SERVICE" name="SERVICE"> | ||||||
| <?= mk_option($libvirt_service, 'disable', _('No'))?> | ||||||
|
|
@@ -150,6 +150,7 @@ _(Disable Autostart/Start option for VMs)_: | |||||
| <?if ($libvirt_up):?> | ||||||
| <?$libvirt_info = libvirt_version('libvirt')?> | ||||||
| <?$qemu_info = $lv->get_connect_information()?> | ||||||
|
|
||||||
| _(Libvirt version)_: | ||||||
| : <?=$libvirt_info['libvirt.major'].'.'.$libvirt_info['libvirt.minor'].'.'.$libvirt_info['libvirt.release']?> | ||||||
|
|
||||||
|
|
@@ -161,14 +162,15 @@ _(Libvirt storage location)_: | |||||
|
|
||||||
| :vms_libvirt_volume_help: | ||||||
|
|
||||||
| <?else: /* Libvirt is stopped */ ?> | ||||||
| _(Libvirt vdisk size)_ (_(GB)_): | ||||||
| <?else : /* Libvirt is stopped */ ?> | ||||||
|
|
||||||
| _(Libvirt vdisk size)_ (_(GB)_): | ||||||
| : <input type="number" id="IMAGE_SIZE" name="IMAGE_SIZE" min="1" value="<?=htmlspecialchars($domain_cfg['IMAGE_SIZE']);?>" required="required" /><span id="SIZE_ERROR" class="errortext"></span> | ||||||
|
|
||||||
| :vms_libvirt_vdisk_size_help: | ||||||
|
|
||||||
| _(Libvirt storage location)_: | ||||||
| : <input type="text" id="IMAGE_FILE" name="IMAGE_FILE" autocomplete="off" spellcheck="false" value="<?=htmlspecialchars($domain_cfg['IMAGE_FILE']);?>" placeholder="e.g. /mnt/user/system/libvirt/libvirt.img" data-pickcloseonfile="true" data-pickfilter="img" data-pickroot="/mnt" data-pickfolders="true" required pattern="^[^\\]*libvirt\.img$"> | ||||||
| : <input type="text" id="IMAGE_FILE" name="IMAGE_FILE" autocomplete="off" spellcheck="false" value="<?=htmlspecialchars($domain_cfg['IMAGE_FILE']);?>" placeholder="e.g. /mnt/user/system/libvirt/libvirt.img" data-pickcloseonfile="true" data-pickfilter="img" data-pickroot="/mnt" data-pickfolders="true" > | ||||||
| <?if (file_exists($domain_cfg['IMAGE_FILE'])):?> | ||||||
| <span id="deletePanel"><label><input type="checkbox" id="deleteCheckbox" /> _(Delete Image File)_</label></span><?endif;?> | ||||||
| <?if (!$started):?> | ||||||
|
|
@@ -179,6 +181,7 @@ _(Libvirt storage location)_: | |||||
| :vms_libvirt_location_help: | ||||||
|
|
||||||
| <?endif;?> | ||||||
|
|
||||||
| _(Default VM storage path)_: | ||||||
| : <input type="text" id="domaindir" name="DOMAINDIR" autocomplete="off" spellcheck="false" data-pickfolders="true" data-pickfilter="HIDE_FILES_FILTER" data-pickroot="/mnt" value="<?=htmlspecialchars($domain_cfg['DOMAINDIR'])?>" placeholder="_(Click to Select)_" pattern="^[^\\]*/$" onchange="validatePath(this)"> | ||||||
| <?if (!$started):?> | ||||||
|
|
@@ -299,7 +302,7 @@ _(VFIO allow unsafe interrupts)_: | |||||
|
|
||||||
| <?endif;?> | ||||||
|
|
||||||
| <?if ($libvirt_up && trim(shell_exec('stat -c %T -f /etc/libvirt'))=='btrfs'):?> | ||||||
| <?if ($libvirt_up && trim(shell_exec('stat -c %T -f /etc/libvirt'))=='btrfs' && strpos($domain_cfg['IMAGE_FILE'],".img")) :?> | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix imprecise string matching for The condition
Since the intent is to show this section only for image files (not directories), consider using a more precise check: -<?if ($libvirt_up && trim(shell_exec('stat -c %T -f /etc/libvirt'))=='btrfs' && strpos($domain_cfg['IMAGE_FILE'],".img")) :?>
+<?if ($libvirt_up && trim(shell_exec('stat -c %T -f /etc/libvirt'))=='btrfs' && substr($domain_cfg['IMAGE_FILE'], -4) === '.img') :?>This checks if the path ends with 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| <div class="advanced" markdown="1"> | ||||||
| <div class="title"><span class="left"><i class="title fa fa-list"></i>_(Libvirt volume info)_</span></div> | ||||||
| _(btrfs filesystem show)_: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* Filesystem helper utilities for dynamix.vm.manager | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - files_identical($a,$b) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - copy_if_different($src,$dst, $dry_run=false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - dir_copy($src,$dst) (recursive, skips identical files) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - dir_remove($dir) (recursive remove) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function files_identical($a, $b) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!file_exists($a) || !file_exists($b)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (filesize($a) !== filesize($b)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $ha = @md5_file($a); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $hb = @md5_file($b); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ($ha === false || $hb === false) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return $ha === $hb; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function copy_if_different($src, $dst, $dry_run = false) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'src' => $src, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'dst' => $dst, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'would_copy' => false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'copied' => false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'error' => null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!file_exists($src)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['error'] = 'source not found'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $dst_dir = dirname($dst); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!is_dir($dst_dir)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ($dry_run) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['would_copy'] = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!@mkdir($dst_dir, 0755, true)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['error'] = 'failed to create dest dir'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (file_exists($dst)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (files_identical($src, $dst)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return $result; // identical, nothing to do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['would_copy'] = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['would_copy'] = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ($dry_run) return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (@copy($src, $dst)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['copied'] = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $result['error'] = 'copy_failed'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function dir_copy($src, $dst) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!is_dir($src)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!is_dir($dst)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!@mkdir($dst, 0755, true)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $items = scandir($src); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach ($items as $item) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ($item === '.' || $item === '..') continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $s = $src . DIRECTORY_SEPARATOR . $item; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $d = $dst . DIRECTORY_SEPARATOR . $item; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (is_dir($s)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!dir_copy($s, $d)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (file_exists($d)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (files_identical($s, $d)) continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!@copy($s, $d)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function dir_remove($dir) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!is_dir($dir)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $items = scandir($dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach ($items as $item) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ($item === '.' || $item === '..') continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $path = $dir . DIRECTORY_SEPARATOR . $item; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (is_dir($path)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dir_remove($path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @unlink($path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return @rmdir($dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recursive removal doesn't propagate child failure status. When Suggested fix to propagate failures function dir_remove($dir) {
if (!is_dir($dir)) return false;
$items = scandir($dir);
+ if ($items === false) return false;
foreach ($items as $item) {
if ($item === '.' || $item === '..') continue;
$path = $dir . DIRECTORY_SEPARATOR . $item;
if (is_dir($path)) {
- dir_remove($path);
+ if (!dir_remove($path)) return false;
} else {
- @unlink($path);
+ if (!@unlink($path)) return false;
}
}
return @rmdir($dir);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,64 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
| # run & log functions | ||||||||||||||||||||||||||||||||||||||||||||||||
| . /etc/rc.d/rc.runlog | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Sync domain data if IMAGE_FILE and OLD_IMAGE_FILE differ | ||||||||||||||||||||||||||||||||||||||||||||||||
| DOMAIN_CFG=/boot/config/domain.cfg | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Read values from domain.cfg | ||||||||||||||||||||||||||||||||||||||||||||||||
| eval $(grep -E '^(IMAGE_FILE|OLD_IMAGE_FILE)=' "$DOMAIN_CFG") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Remove quotes | ||||||||||||||||||||||||||||||||||||||||||||||||
| IMAGE_FILE="${IMAGE_FILE%\"}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| IMAGE_FILE="${IMAGE_FILE#\"}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| OLD_IMAGE_FILE="${OLD_IMAGE_FILE%\"}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| OLD_IMAGE_FILE="${OLD_IMAGE_FILE#\"}" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential command injection via Using Safer alternative using grep/sed-# Read values from domain.cfg
-eval $(grep -E '^(IMAGE_FILE|OLD_IMAGE_FILE)=' "$DOMAIN_CFG")
+# Read values from domain.cfg safely (no eval)
+IMAGE_FILE=$(grep -E '^IMAGE_FILE=' "$DOMAIN_CFG" | head -1 | cut -d= -f2-)
+OLD_IMAGE_FILE=$(grep -E '^OLD_IMAGE_FILE=' "$DOMAIN_CFG" | head -1 | cut -d= -f2-)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Proceed only if both variables are set and OLD_IMAGE_FILE exists | ||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -n "$IMAGE_FILE" ] && [ -n "$OLD_IMAGE_FILE" ] && [ "$IMAGE_FILE" != "$OLD_IMAGE_FILE" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||
| if [ ! -e "$OLD_IMAGE_FILE" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "OLD_IMAGE_FILE not found: $OLD_IMAGE_FILE — skipping sync" | ||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "IMAGE_FILE and OLD_IMAGE_FILE differ, syncing..." | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| TMP_MNT=/etc/libvirt-sync | ||||||||||||||||||||||||||||||||||||||||||||||||
| IMG_FILE_NAME=$(basename "$IMAGE_FILE") | ||||||||||||||||||||||||||||||||||||||||||||||||
| OLD_IMG_FILE_NAME=$(basename "$OLD_IMAGE_FILE") | ||||||||||||||||||||||||||||||||||||||||||||||||
| TIMESTAMP=$(date +%Y%m%d-%H%M%S) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "$OLD_IMAGE_FILE" == *.img ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Backup image before mounting | ||||||||||||||||||||||||||||||||||||||||||||||||
| BACKUP_PATH="${OLD_IMAGE_FILE%.img}.bak-${TIMESTAMP}.img" | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "Creating backup of OLD_IMAGE_FILE: $BACKUP_PATH" | ||||||||||||||||||||||||||||||||||||||||||||||||
| cp -p "$OLD_IMAGE_FILE" "$BACKUP_PATH" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| log "Mounting $OLD_IMAGE_FILE to $TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "$TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| mount "$OLD_IMAGE_FILE" "$TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "Copying full contents from image to directory $IMAGE_FILE" | ||||||||||||||||||||||||||||||||||||||||||||||||
| rsync -a --exclude="$OLD_IMG_FILE_NAME" "$TMP_MNT/" "$IMAGE_FILE/" | ||||||||||||||||||||||||||||||||||||||||||||||||
| umount "$TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mount operations lack error checking; failure leaves stale mount point. If Suggested hardening with error checks log "Mounting $OLD_IMAGE_FILE to $TMP_MNT"
mkdir -p "$TMP_MNT"
- mount "$OLD_IMAGE_FILE" "$TMP_MNT"
+ if ! mount "$OLD_IMAGE_FILE" "$TMP_MNT"; then
+ log "ERROR: Failed to mount $OLD_IMAGE_FILE"
+ rm -rf "$TMP_MNT"
+ exit 1
+ fi
log "Copying full contents from image to directory $IMAGE_FILE"
- rsync -a --exclude="$OLD_IMG_FILE_NAME" "$TMP_MNT/" "$IMAGE_FILE/"
- umount "$TMP_MNT"
+ if ! rsync -a --exclude="$OLD_IMG_FILE_NAME" "$TMP_MNT/" "$IMAGE_FILE/"; then
+ log "WARNING: rsync encountered errors"
+ fi
+ umount "$TMP_MNT" || log "WARNING: Failed to unmount $TMP_MNT"
+ rmdir "$TMP_MNT" 2>/dev/nullApply similar error handling to the other mount/umount blocks (lines 46-51). 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| elif [[ "$IMAGE_FILE" == *.img ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "Mounting $IMAGE_FILE to $TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "$TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| mount "$IMAGE_FILE" "$TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "Copying full contents from directory $OLD_IMAGE_FILE to image" | ||||||||||||||||||||||||||||||||||||||||||||||||
| rsync -a --exclude="$IMG_FILE_NAME" --exclude='*.bak-*.img' "$OLD_IMAGE_FILE/" "$TMP_MNT/" | ||||||||||||||||||||||||||||||||||||||||||||||||
| umount "$TMP_MNT" | ||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "Both IMAGE_FILE and OLD_IMAGE_FILE are directories, copying full contents" | ||||||||||||||||||||||||||||||||||||||||||||||||
| rsync -a --exclude="$IMG_FILE_NAME" "$OLD_IMAGE_FILE/" "$IMAGE_FILE/" | ||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Update OLD_IMAGE_FILE in domain.cfg | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "Updating OLD_IMAGE_FILE in $DOMAIN_CFG" | ||||||||||||||||||||||||||||||||||||||||||||||||
| sed -i "s|^OLD_IMAGE_FILE=.*|OLD_IMAGE_FILE=\"$IMAGE_FILE\"|" "$DOMAIN_CFG" | ||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "IMAGE_FILE and OLD_IMAGE_FILE match, or one is unset — skipping sync" | ||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # missing qemu directory would indicate new libvirt image file created | ||||||||||||||||||||||||||||||||||||||||||||||||
| if [ ! -d /etc/libvirt/qemu ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||
| log "initializing /etc/libvirt" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -36,3 +94,7 @@ if [ -s /var/log/vfio-pci-errors ]; then | |||||||||||||||||||||||||||||||||||||||||||||||
| echo "vfio-pci bind error" > /run/libvirt/qemu/autostarted | ||||||||||||||||||||||||||||||||||||||||||||||||
| /usr/local/emhttp/webGui/scripts/notify -e "VM Autostart disabled" -s "vfio-pci-errors " -d "VM Autostart disabled due to vfio-bind error" -m "Please review /var/log/vfio-pci-errors" -i "alert" -l "/VMs" | ||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Copy XML from VM Directories to QEMU directory/ | ||||||||||||||||||||||||||||||||||||||||||||||||
| /usr/local/emhttp/plugins/dynamix.vm.manager/scripts/libvirtrestore | ||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||
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.
Fix grammar: "at change" is awkward.
The phrase "at change" should be replaced with clearer alternatives. This issue was previously flagged and remains unresolved.
📝 Suggested fix
Or alternatively:
📝 Committable suggestion
🤖 Prompt for AI Agents