Bug 1293271

Summary: virt-sysprep --delete fails to delete a symbolic link to a dir
Product: Red Hat Enterprise Linux 6 Reporter: Xianghua Chen <xchen>
Component: libguestfsAssignee: Pino Toscano <ptoscano>
Status: CLOSED WONTFIX QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 6.8CC: leiwang, linl, ptoscano, rjones, wshi, xchen
Target Milestone: rcKeywords: Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-11-13 14:02:14 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1301844    
Attachments:
Description Flags
log for virt-sysprep --delete none

Description Xianghua Chen 2015-12-21 09:34:52 UTC 
Created attachment 1108267 [details]
log for virt-sysprep --delete

Description of problem:
I was trying to use  virt-sysprep --delete to delete a  symbolic link to a dir:'/home/testdir_link' in guestos, but it failed without any prompt.
Then I tried it in RHEL7.2(libguestfs-1.28.1-55.el7.x86_64), it's ok.
I looked into the two detailed log and found that in RHEL7.2 it used 'rm_rf "/home/testdir_link"',but in RHEL6.8 it used 'rm_rf "/home/testdir_link/"' , which caused the failure.


Version-Release number of selected component (if applicable):
libguestfs-1.20.11-16.el6.x86_64
libguestfs-tools-c-1.20.11-16.el6.x86_64


How reproducible:
100%

Steps to Reproduce:
1. Prepare a image: RHEL-Server-6.7-32-hvm.raw
2.
$ guestfish -a RHEL-Server-6.7-32-hvm.raw -i mkdir /home/testdir
$ guestfish -a RHEL-Server-6.7-32-hvm.raw -i ln-s /home/testdir /home/testdir_link
$ virt-sysprep -a RHEL-Server-6.7-32-hvm.raw --delete /home/testdir_link -v -x 2>&1 | tee tmplog
$ guestfish -a RHEL-Server-6.7-32-hvm.raw -i glob-expand '/home/testdir_link'
/home/testdir_link/
3. Then I tried in guestfish and found that you can only use "rm-rf /home/testdir_link" to delete it (it's same as the host):
$ guestfish -a RHEL-Server-6.7-32-hvm.raw -i
><fs> rm-rf /home/testdir_link/
><fs> ls /home/
testdir
testdir_link
><fs> rm-rf /home/testdir_link
><fs> ls /home/
testdir


Actual results:
After step 2, the  "/home/testdir_link/" is still there.

Expected results:
It should be deleted successfully.

Additional info:
RHEL7.2(libguestfs-1.28.1-55.el7.x86_64) is ok.
Comment 1 Xianghua Chen 2016-01-04 06:48:54 UTC 

*** This bug has been marked as a duplicate of bug 1216296 ***
Comment 2 Richard W.M. Jones 2016-01-04 12:56:06 UTC 
I'm unduplicating this, because it doesn't really have
anything to do with the rm-rf API.

What happens is that we call glob_expand first, and that API
returns the path with trailing "/":

libguestfs: trace: glob_expand "/home/testdir_link"
libguestfs: trace: glob_expand = ["/home/testdir_link/"]
libguestfs: trace: rm_rf "/home/testdir_link/"

What's interesting is:

(1) This does not happen upstream.  Upstream, the virt-sysprep
--delete option does not support globbing at all, which I think
may be an accidental regression.

(2) This does not happen in upstream 1.20 either.

It only happens in RHEL 6 because we backported the following
commits:

https://github.com/libguestfs/libguestfs/commit/405263ab9642bb025b5af699de1b1599f7b0ffce
https://github.com/libguestfs/libguestfs/commit/3bebe598abaa490c6f0743b8fb0447e113913ac3

(The second commit is just a doc change).
Comment 3 Xianghua Chen 2016-01-05 02:24:42 UTC 
(In reply to Richard W.M. Jones from comment #2)
> I'm unduplicating this, because it doesn't really have
> anything to do with the rm-rf API.
> 
> What happens is that we call glob_expand first, and that API
> returns the path with trailing "/":
> 
> libguestfs: trace: glob_expand "/home/testdir_link"
> libguestfs: trace: glob_expand = ["/home/testdir_link/"]
> libguestfs: trace: rm_rf "/home/testdir_link/"
> 
> What's interesting is:
> 
> (1) This does not happen upstream.  Upstream, the virt-sysprep
> --delete option does not support globbing at all, which I think
> may be an accidental regression.
> 
> (2) This does not happen in upstream 1.20 either.
> 
> It only happens in RHEL 6 because we backported the following
> commits:
> 
> https://github.com/libguestfs/libguestfs/commit/
> 405263ab9642bb025b5af699de1b1599f7b0ffce
> https://github.com/libguestfs/libguestfs/commit/
> 3bebe598abaa490c6f0743b8fb0447e113913ac3
> 
> (The second commit is just a doc change).

Yes, I agree with you, it has nothing to do with rm-rf API, but I looked into the steps of the  bug 1216296, it's actually the same. 
So I duplicate it because they are the same problem, may be the subject of  bug 1216296 is not proper.  I'll close the  bug 1216296 as duplicate if it confused you.
Comment 6 Pino Toscano 2016-01-15 15:11:47 UTC 
The situation described in this bug is actually a bit more convoluted than it seems:

(1) --delete in RHEL 7 and upstream does not support globbing; this has been introduced during the creation of virt-customize, when virt-sysprep has been changed to use the customize operations, losing the globbing feature of --delete

(2) when globbing is used, glob_expand returns paths with trailing slash, and `rm -rf dir-symlink/` does not work; this happens because in the glob_expand implementation we pass GLOB_MARK to glob()

(3) `rm -rf dir-symlink/` does not remove that symlink to a directory (whereas without the slash it works); this is also bug #1216296

So:

- regarding (1): implementing globbing for --delete is trivial, but then it would introduce (2) upstream as well

- removing GLOB_MARK from the flags of glob() fixes (2), although that may break users of the glob_expand API (i.e. those relying on directories ending with slash, and not appending it by themselves)
Comment 7 Xianghua Chen 2016-02-01 07:28:05 UTC 
*** Bug 1216296 has been marked as a duplicate of this bug. ***
Comment 9 Richard W.M. Jones 2017-11-13 14:02:14 UTC 
I'm closing this as WONTFIX for a couple of reasons:

(1) RHEL 6 is in production phase 3.

(2) This wasn't reported by a customer.

Note that this does not mean this is not a bug, just that we are not
going to fix it in RHEL 6.